From aa15bdaacbb762bbb0d9bd10d24cef147b5dd029 Mon Sep 17 00:00:00 2001 From: "G.Ambatte" Date: Thu, 10 Jul 2025 19:59:09 +1200 Subject: [PATCH 01/10] Initial pass at ID validations --- .../pages/errorPage/errors/errorIndex.js | 20 +++++++++++++++++++ server/homebrew.api.js | 9 +++++++++ 2 files changed, 29 insertions(+) diff --git a/client/homebrew/pages/errorPage/errors/errorIndex.js b/client/homebrew/pages/errorPage/errors/errorIndex.js index b13b19eb1..c0220b648 100644 --- a/client/homebrew/pages/errorPage/errors/errorIndex.js +++ b/client/homebrew/pages/errorPage/errors/errorIndex.js @@ -176,6 +176,26 @@ const errorIndex = (props)=>{ If the selected brew is your document, you may designate it as a theme by adding the \`theme:meta\` tag.`, + // ID validation error + '11' : dedent` + ## No Homebrewery document could be found. + + The server could not locate the Homebrewery document. The Brew ID failed the validation check. + + : + + **Brew ID:** ${props.brew.brewId}`, + + // Google ID validation error + '12' : dedent` + ## No Google document could be found. + + The server could not locate the Google document. The Google ID failed the validation check. + + : + + **Brew ID:** ${props.brew.brewId}`, + //account page when account is not defined '50' : dedent` ## You are not signed in diff --git a/server/homebrew.api.js b/server/homebrew.api.js index 84f639a4d..fc62ac763 100644 --- a/server/homebrew.api.js +++ b/server/homebrew.api.js @@ -48,6 +48,15 @@ const api = { } id = id.slice(googleId.length); } + + // ID Validation Checks + if(!id.match(/^[A-Za-z0-9_-]{12}$/)){ + throw { name: 'ID Error', message: 'Invalid ID', status: 404, HBErrorCode: '11', brewId: id }; + } + if(googleId && !googleId.match(/^1(?:[A-Za-z0-9+\/]{32}|[A-Za-z0-9+\/]{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` From 25f25da4995cced81df49eee5f0974480dbb15c4 Mon Sep 17 00:00:00 2001 From: "G.Ambatte" Date: Thu, 10 Jul 2025 20:39:12 +1200 Subject: [PATCH 02/10] Adjust validation regex for IDs --- server/homebrew.api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/homebrew.api.js b/server/homebrew.api.js index fc62ac763..0a62ac298 100644 --- a/server/homebrew.api.js +++ b/server/homebrew.api.js @@ -50,7 +50,7 @@ const api = { } // ID Validation Checks - if(!id.match(/^[A-Za-z0-9_-]{12}$/)){ + if(!id.match(/^[A-Za-z0-9_-]{7,12}$/)){ throw { name: 'ID Error', message: 'Invalid ID', status: 404, HBErrorCode: '11', brewId: id }; } if(googleId && !googleId.match(/^1(?:[A-Za-z0-9+\/]{32}|[A-Za-z0-9+\/]{43})$/)){ From 1794e96d50f4644417a60170552ace6a3122bf80 Mon Sep 17 00:00:00 2001 From: "G.Ambatte" Date: Thu, 10 Jul 2025 20:46:01 +1200 Subject: [PATCH 03/10] Update tests --- server/homebrew.api.spec.js | 41 +++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/server/homebrew.api.spec.js b/server/homebrew.api.spec.js index 8bb3a0c0b..1de46b455 100644 --- a/server/homebrew.api.spec.js +++ b/server/homebrew.api.spec.js @@ -99,18 +99,51 @@ 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', ()=>{ + 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 return 12-char id and google id from params', ()=>{ From abef25063140d9eb3f53d2f3dbf696357b789532 Mon Sep 17 00:00:00 2001 From: "G.Ambatte" Date: Thu, 10 Jul 2025 20:58:46 +1200 Subject: [PATCH 04/10] Update ID validation check --- server/homebrew.api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/homebrew.api.js b/server/homebrew.api.js index 0a62ac298..842f8a9e6 100644 --- a/server/homebrew.api.js +++ b/server/homebrew.api.js @@ -50,7 +50,7 @@ const api = { } // ID Validation Checks - if(!id.match(/^[A-Za-z0-9_-]{7,12}$/)){ + if(!id.match(/^[A-Za-z0-9_-]{7,14}$/)){ throw { name: 'ID Error', message: 'Invalid ID', status: 404, HBErrorCode: '11', brewId: id }; } if(googleId && !googleId.match(/^1(?:[A-Za-z0-9+\/]{32}|[A-Za-z0-9+\/]{43})$/)){ From 677c02cfa530ab29b2828460d088ce25dab5b206 Mon Sep 17 00:00:00 2001 From: "G.Ambatte" Date: Sun, 13 Jul 2025 22:36:53 +1200 Subject: [PATCH 05/10] Add forceSSL tests --- server/forcessl.mw.spec.js | 74 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 server/forcessl.mw.spec.js diff --git a/server/forcessl.mw.spec.js b/server/forcessl.mw.spec.js new file mode 100644 index 000000000..c22954f68 --- /dev/null +++ b/server/forcessl.mw.spec.js @@ -0,0 +1,74 @@ +import forceSSL from './forcessl.mw'; + +describe('Tests for ForceSSL middleware', ()=>{ + + it('should call next() when NODE_ENV is set to local', ()=>{ + const nodeEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'local'; + + const nextFn = jest.fn(); + + forceSSL(null, null, nextFn); + + process.env.NODE_ENV = nodeEnv; + + expect(nextFn).toHaveBeenCalled(); + }); + + it('should call next() when NODE_ENV is set to docker', ()=>{ + const nodeEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'docker'; + + const nextFn = jest.fn(); + + forceSSL(null, null, nextFn); + + process.env.NODE_ENV = nodeEnv; + + expect(nextFn).toHaveBeenCalled(); + }); + + it('should return 302 when header not HTTPS', ()=>{ + const nodeEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'test'; + + const req = { + header : ()=>{ return true; }, + get : ()=>{ return 'test'; }, + url : 'URL' + }; + + const res = { + redirect : jest.fn((code, url)=>{}) + }; + + const nextFn = jest.fn(); + + forceSSL(req, res, nextFn); + + process.env.NODE_ENV = nodeEnv; + + expect(res.redirect).toHaveBeenCalledWith(302, 'https://testURL'); + }); + + it('should call next() header is HTTPS and NODE_ENV not local or docker', ()=>{ + const nodeEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'test'; + + const req = { + header : ()=>{ return 'https'; } + }; + + const res = { + }; + + const nextFn = jest.fn(); + + forceSSL(req, res, nextFn); + + process.env.NODE_ENV = nodeEnv; + + expect(nextFn).toHaveBeenCalled(); + }); + +}); \ No newline at end of file From 40839b18e47694c5839b3cc56f86389c13b0d910 Mon Sep 17 00:00:00 2001 From: "G.Ambatte" Date: Mon, 14 Jul 2025 00:14:58 +1200 Subject: [PATCH 06/10] Add tests for token.js --- server/token.spec.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 server/token.spec.js diff --git a/server/token.spec.js b/server/token.spec.js new file mode 100644 index 000000000..24ebb7f7c --- /dev/null +++ b/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'); + }); +}); \ No newline at end of file From 248d2038ece9dd15d2215dcda1100324cdc5573c Mon Sep 17 00:00:00 2001 From: Trevor Buckner Date: Mon, 14 Jul 2025 13:10:19 -0400 Subject: [PATCH 07/10] Cleanup token.js --- server/token.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/server/token.js b/server/token.js index 7a23dff4b..feaea8d33 100644 --- a/server/token.js +++ b/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; \ No newline at end of file +export default generateAccessToken; From 973e071e930461560252e6493620780edbb31474 Mon Sep 17 00:00:00 2001 From: "G.Ambatte" Date: Tue, 15 Jul 2025 08:13:35 +1200 Subject: [PATCH 08/10] Slightly loosen Google ID match criteria, add comments --- server/homebrew.api.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/server/homebrew.api.js b/server/homebrew.api.js index bd8764b51..82d64c1a3 100644 --- a/server/homebrew.api.js +++ b/server/homebrew.api.js @@ -50,10 +50,15 @@ const api = { } // 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 }; } - if(googleId && !googleId.match(/^1(?:[A-Za-z0-9+\/]{32}|[A-Za-z0-9+\/]{43})$/)){ + // 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 }; } From 828208aadb9b71a90a3e545f0c27728d1f81296e Mon Sep 17 00:00:00 2001 From: "G.Ambatte" Date: Tue, 15 Jul 2025 08:19:05 +1200 Subject: [PATCH 09/10] Add more ID validation test cases --- server/homebrew.api.spec.js | 38 ++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/server/homebrew.api.spec.js b/server/homebrew.api.spec.js index 0b57588ea..0a6d1d452 100644 --- a/server/homebrew.api.spec.js +++ b/server/homebrew.api.spec.js @@ -128,7 +128,7 @@ describe('Tests for api', ()=>{ expect(googleId).toEqual('123456789012345678901234567890123'); }); - it('should throw invalid google id', ()=>{ + it('should throw invalid - google id right length but does not match pattern', ()=>{ let err; try { api.getId({ @@ -146,6 +146,42 @@ describe('Tests for api', ()=>{ 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', ()=>{ const { id, googleId } = api.getId({ params : { From 90e577dd3fd0e45023c8a64fe05a52957e492470 Mon Sep 17 00:00:00 2001 From: "G.Ambatte" Date: Tue, 15 Jul 2025 09:02:57 +1200 Subject: [PATCH 10/10] Rework tests --- server/forcessl.mw.spec.js | 96 +++++++++++++++++--------------------- 1 file changed, 44 insertions(+), 52 deletions(-) diff --git a/server/forcessl.mw.spec.js b/server/forcessl.mw.spec.js index c22954f68..e18821e6d 100644 --- a/server/forcessl.mw.spec.js +++ b/server/forcessl.mw.spec.js @@ -1,73 +1,65 @@ import forceSSL from './forcessl.mw'; describe('Tests for ForceSSL middleware', ()=>{ + let originalEnv; + let nextFn; - it('should call next() when NODE_ENV is set to local', ()=>{ - const nodeEnv = process.env.NODE_ENV; - process.env.NODE_ENV = 'local'; + let req = {}; + let res = {}; - const nextFn = jest.fn(); + beforeEach(()=>{ + originalEnv = process.env.NODE_ENV; + nextFn = jest.fn(); - forceSSL(null, null, nextFn); - - process.env.NODE_ENV = nodeEnv; - - expect(nextFn).toHaveBeenCalled(); - }); - - it('should call next() when NODE_ENV is set to docker', ()=>{ - const nodeEnv = process.env.NODE_ENV; - process.env.NODE_ENV = 'docker'; - - const nextFn = jest.fn(); - - forceSSL(null, null, nextFn); - - process.env.NODE_ENV = nodeEnv; - - expect(nextFn).toHaveBeenCalled(); - }); - - it('should return 302 when header not HTTPS', ()=>{ - const nodeEnv = process.env.NODE_ENV; - process.env.NODE_ENV = 'test'; - - const req = { - header : ()=>{ return true; }, + req = { + header : ()=>{ return 'http'; }, get : ()=>{ return 'test'; }, url : 'URL' }; - const res = { - redirect : jest.fn((code, url)=>{}) + res = { + redirect : jest.fn() }; - - const nextFn = jest.fn(); - - forceSSL(req, res, nextFn); - - process.env.NODE_ENV = nodeEnv; - - expect(res.redirect).toHaveBeenCalledWith(302, 'https://testURL'); + }); + afterEach(()=>{ + process.env.NODE_ENV = originalEnv; + jest.clearAllMocks(); }); - it('should call next() header is HTTPS and NODE_ENV not local or docker', ()=>{ - const nodeEnv = process.env.NODE_ENV; + 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'; - const req = { - header : ()=>{ return 'https'; } - }; - - const res = { - }; - - const nextFn = jest.fn(); - forceSSL(req, res, nextFn); - process.env.NODE_ENV = nodeEnv; + 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(); });