From 8b867bd35db000726c0a8fdaa8b68cfe9fbfe06f Mon Sep 17 00:00:00 2001 From: Nick Downing Date: Mon, 17 Jan 2022 10:34:09 +1100 Subject: [PATCH] Add expiry on sign up draft (to prevent leakage of personal information) --- _lib/session_cookie.jst | 10 ++-- api/account/sign_up/create_account.json.jst | 12 ++++- api/account/sign_up/get_draft.json.jst | 28 +++++++++-- api/account/sign_up/set_draft.json.jst | 33 ++++++++++--- my_account/sign_up/index.html.jst | 51 +++++++++++++++------ 5 files changed, 103 insertions(+), 31 deletions(-) diff --git a/_lib/session_cookie.jst b/_lib/session_cookie.jst index d5b87a6..01618ea 100644 --- a/_lib/session_cookie.jst +++ b/_lib/session_cookie.jst @@ -4,13 +4,17 @@ let crypto = require('crypto') let XDate = require('xdate') return async (env, transaction) => { - let cookies = cookie.parse(env.request.headers.cookie || '') - let now = Date.now() - let sessions = await ( await transaction.get({}) ).get('sessions', {}) + // do not generate a new session key for multiple calls per request + if (env.session_key !== undefined) + return /*await*/ sessions.get(env.session_key, {}) + + let cookies = cookie.parse(env.request.headers.cookie || '') + let now = Date.now() + let session, expires = new XDate(now) if ( Object.prototype.hasOwnProperty.call(cookies, 'session_key') && diff --git a/api/account/sign_up/create_account.json.jst b/api/account/sign_up/create_account.json.jst index 85c4ef7..db66bb9 100644 --- a/api/account/sign_up/create_account.json.jst +++ b/api/account/sign_up/create_account.json.jst @@ -1,3 +1,4 @@ +let logjson = (await import('@ndcode/logjson')).default let XDate = require('xdate') return async env => { @@ -36,14 +37,21 @@ return async env => { let session = await session_cookie(env, transaction) let captcha = await session.get('captcha') - if (captcha === undefined || XDate.now() >= captcha.get('expires')) + if ( + captcha === undefined || + XDate.now() >= await logjson.logjson_to_json( + await captcha.get('expires') + ) + ) throw new Problem( 'No verification image in session', `Please call the "/api/verification_image.png" endpoint to create a verification image, in same session as the "/api/account/sign_up/create_account.json" call and less than one hour prior.`, 418 ) - let captcha_text = await captcha.get('text') + let captcha_text = await logjson.logjson_to_json( + await captcha.get('text') + ) if (verification_code !== captcha_text) { console.log(`verification code mismatch, \"${verification_code}\" should be \"${captcha_text}\"`) diff --git a/api/account/sign_up/get_draft.json.jst b/api/account/sign_up/get_draft.json.jst index 197fd24..b07ab2b 100644 --- a/api/account/sign_up/get_draft.json.jst +++ b/api/account/sign_up/get_draft.json.jst @@ -16,12 +16,32 @@ return async env => { // initialize env.session_key, set cookie in env.response let session = await session_cookie(env, transaction) - return await logjson.logjson_to_json( - await session.get('sign_up_draft', {}) - ) + let sign_up_draft = await session.get('sign_up_draft') + let details = + sign_up_draft !== undefined && + XDate.now() < await logjson.logjson_to_json( + await sign_up_draft.get('expires') + ) ? { + email: await logjson.logjson_to_json( + await sign_up_draft.get('email') + ), + given_names: await logjson.logjson_to_json( + await sign_up_draft.get('given_names') + ), + family_name: await logjson.logjson_to_json( + await sign_up_draft.get('family_name') + ), + contact_me: await logjson.logjson_to_json( + await sign_up_draft.get('contact_me') + ) + } : null + + await transaction.commit() + return details } - finally { + catch (error) { transaction.rollback() + throw error } } ) diff --git a/api/account/sign_up/set_draft.json.jst b/api/account/sign_up/set_draft.json.jst index f7ca03b..0e3c105 100644 --- a/api/account/sign_up/set_draft.json.jst +++ b/api/account/sign_up/set_draft.json.jst @@ -11,19 +11,38 @@ return async env => { // handler async details => { // coerce and/or validate - details = { - email: details.email.slice(0, 256).toLowerCase(), - given_names: details.given_names.slice(0, 256), - family_name: details.family_name.slice(0, 256), - contact_me: details.contact_me ? true : false - } + if (details !== null) + details = { + email: details.email.slice(0, 256).toLowerCase(), + given_names: details.given_names.slice(0, 256), + family_name: details.family_name.slice(0, 256), + contact_me: details.contact_me ? true : false + } let transaction = await env.site.database.Transaction() try { // initialize env.session_key, set cookie in env.response let session = await session_cookie(env, transaction) - session.set('sign_up_draft', transaction.json_to_logjson(details)) + if (details) { + let expires = new XDate() + expires.addDays(1) + session.set( + 'sign_up_draft', + transaction.json_to_logjson( + { + email: details.email, + given_names: details.given_names, + family_name: details.family_name, + contact_me: details.contact_me, + expires: expires.getTime() + } + ) + ) + } + else + session.delete('sign_up_draft') + await transaction.commit() } catch (error) { diff --git a/my_account/sign_up/index.html.jst b/my_account/sign_up/index.html.jst index 228b5b0..295b238 100644 --- a/my_account/sign_up/index.html.jst +++ b/my_account/sign_up/index.html.jst @@ -1,4 +1,5 @@ let logjson = (await import('@ndcode/logjson')).default +let XDate = require('xdate') return async env => { let breadcrumbs = await _require('/_lib/breadcrumbs.jst') @@ -8,19 +9,38 @@ return async env => { let session_cookie = await _require('/_lib/session_cookie.jst') // preload draft details if any - let transaction = await env.site.database.Transaction(), details + let transaction = await env.site.database.Transaction(), draft_details try { // initialize env.session_key, set cookie in env.response let session = await session_cookie(env, transaction) - details = await logjson.logjson_to_json( - await session.get('sign_up_draft', {}) - ) + let sign_up_draft = await session.get('sign_up_draft') + draft_details = + sign_up_draft !== undefined && + XDate.now() < await logjson.logjson_to_json( + await sign_up_draft.get('expires') + ) ? { + email: await logjson.logjson_to_json( + await sign_up_draft.get('email') + ), + given_names: await logjson.logjson_to_json( + await sign_up_draft.get('given_names') + ), + family_name: await logjson.logjson_to_json( + await sign_up_draft.get('family_name') + ), + contact_me: await logjson.logjson_to_json( + await sign_up_draft.get('contact_me') + ) + } : null + + await transaction.commit() } - finally { + catch (error) { transaction.rollback() + throw error } - console.log('details', JSON.stringify(details)) + console.log('draft_details', JSON.stringify(draft_details)) await navbar( env, @@ -32,6 +52,8 @@ return async env => { p {'Signing up allows you to leave comments on our blog and receive communications from us.'} + p {'Your given names are visible to other users if you comment on our blog. Your email and family name remain private. If your name is one word or does not fit given names/family name pattern, then please enter given names only.'} + div.accordion#accordion.mb-5(role="tablist" aria-multiselectable="true") { div.card#step-1 { div.card-header#step-1-heading(role="tab") { @@ -58,13 +80,13 @@ return async env => { div.col-md-6 { div.form-group { label.form-label(for="given-names") {'Given names *'} - input.form-control#given-names(type="text" value=details.given_names || '' placeholder="Your given names" required="required" maxlength=256) {} + input.form-control#given-names(type="text" value=draft_details ? draft_details.given_names : '' placeholder="Your given names" required="required" maxlength=256) {} } } div.col-md-6 { div.form-group { label.form-label(for="family-name") {'Family name'} - input.form-control#family-name(type="text" value=details.family_name || '' placeholder="Your family name" maxlength=256) {} + input.form-control#family-name(type="text" value=draft_details ? draft_details.family_name : '' placeholder="Your family name" maxlength=256) {} } } } @@ -72,7 +94,7 @@ return async env => { div.col-md-6 { div.form-group { label.form-label(for="email") {'Email *'} - input.form-control#email(type="email" value=details.email || '' placeholder="Your email address" required="required" maxlength=256) {} + input.form-control#email(type="email" value=draft_details ? draft_details.email : '' placeholder="Your email address" required="required" maxlength=256) {} } } div.col-md-6 { @@ -85,7 +107,7 @@ return async env => { div.row { div.col-md-12 { div.custom-control.custom-checkbox { - if (details.contact_me === undefined || details.contact_me) + if (!draft_details || draft_details.contact_me) input.custom-control-input#contact-me(type="checkbox" checked="checked") {} else input.custom-control-input#contact-me(type="checkbox") {} @@ -96,7 +118,7 @@ return async env => { } } } - div.row.align-items-center { + div.row.align-items-center.mb-3 { div.'col-md-6' { div.form-group { label.form-label(for="verification-code") {'Verification code *'} @@ -111,9 +133,8 @@ return async env => { } } - p.mt-3 {'Note: If your name is one word or does not fit given names/family name pattern, then please enter given names only. Your given names are visible to other users if you comment on our blog. Your email and family name remain private.'} - button.btn.btn-success#step-1-continue(type="button") {'Continue'} + p.'mt-3'.mb-0 {'* These fields are required.'} } } @@ -267,7 +288,7 @@ return async env => { new Problem( // title 'Bad request', - // details + // detail (error.stack || error.message).toString() // status 400 @@ -304,7 +325,7 @@ return async env => { new Problem( // title 'Bad request', - // details + // detail (error.stack || error.message).toString() // status 400 -- 2.34.1