Skip to content

Commit 2c46822

Browse files
committed
fix: accept base64-encoded nonces for backward compatibility
Old EmailEngine versions generated OAuth nonces using standard base64 encoding. The recent security fix only accepted base64url characters, causing valid cached nonces (within 24-hour TTL) to fail with "Invalid nonce format" errors. Update validation regex to accept both base64 (+, /, =) and base64url (-, _) characters while new nonce generation continues using base64url.
1 parent 40b590b commit 2c46822

File tree

4 files changed

+57
-29
lines changed

4 files changed

+57
-29
lines changed

lib/routes-ui.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4435,8 +4435,8 @@ ${Buffer.from(data.content, 'base64url').toString('base64')}
44354435

44364436
const nonce = data.n || crypto.randomBytes(NONCE_BYTES).toString('base64url');
44374437

4438-
// Validate nonce format (base64url, 21-22 chars)
4439-
if (!/^[A-Za-z0-9_-]{21,22}$/.test(nonce)) {
4438+
// Validate nonce format (base64url, 21-22 chars; also accept base64 for backward compatibility)
4439+
if (!/^[A-Za-z0-9_\-+/]{21,22}={0,2}$/.test(nonce)) {
44404440
let error = Boom.boomify(new Error('Invalid nonce format. Please generate a new authentication URL.'), { statusCode: 400 });
44414441
throw error;
44424442
}

lib/ui-routes/account-routes.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -520,8 +520,8 @@ function init(args) {
520520

521521
const nonce = data.n || crypto.randomBytes(NONCE_BYTES).toString('base64url');
522522

523-
// Validate nonce format (base64url, 21-22 chars)
524-
if (!/^[A-Za-z0-9_-]{21,22}$/.test(nonce)) {
523+
// Validate nonce format (base64url, 21-22 chars; also accept base64 for backward compatibility)
524+
if (!/^[A-Za-z0-9_\-+/]{21,22}={0,2}$/.test(nonce)) {
525525
let error = Boom.boomify(new Error('Invalid nonce format. Please generate a new authentication URL.'), { statusCode: 400 });
526526
throw error;
527527
}

test/oauth-nonce-test.js

Lines changed: 51 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ const path = require('path');
99
const { NONCE_BYTES } = require('../lib/consts');
1010

1111
// Regex that validates OAuth state nonces (from workers/api.js security fix)
12-
const NONCE_VALIDATION_REGEX = /^[A-Za-z0-9_-]{21,22}$/;
12+
// Accepts both base64url and base64 encoding for backward compatibility
13+
const NONCE_VALIDATION_REGEX = /^[A-Za-z0-9_\-+/]{21,22}={0,2}$/;
1314

1415
test('OAuth nonce encoding tests', async t => {
1516
await t.test('base64url encoded nonce matches validation regex', async () => {
@@ -20,40 +21,39 @@ test('OAuth nonce encoding tests', async t => {
2021
}
2122
});
2223

23-
await t.test('standard base64 encoded nonce fails validation regex', async () => {
24-
// Standard base64 can contain +, /, and = which are not in the validation regex
25-
let foundInvalidChar = false;
26-
for (let i = 0; i < 1000 && !foundInvalidChar; i++) {
24+
await t.test('standard base64 encoded nonce passes validation regex', async () => {
25+
// Standard base64 can contain +, /, and = which are now accepted for backward compatibility
26+
for (let i = 0; i < 100; i++) {
2727
const nonce = crypto.randomBytes(NONCE_BYTES).toString('base64');
28-
if (!NONCE_VALIDATION_REGEX.test(nonce)) {
29-
foundInvalidChar = true;
30-
}
28+
assert.ok(NONCE_VALIDATION_REGEX.test(nonce), `base64 nonce should match validation regex: ${nonce}`);
3129
}
32-
assert.ok(foundInvalidChar, 'standard base64 should eventually produce characters that fail validation (+=/) ');
3330
});
3431

35-
await t.test('no NONCE_BYTES usage with plain base64 encoding in codebase', async () => {
36-
// Static analysis: scan source files for incorrect nonce encoding
37-
// This prevents regression of the base64 vs base64url bug
32+
await t.test('new nonce generation uses base64url encoding in codebase', async () => {
33+
// Static analysis: scan source files for nonce generation
34+
// New nonces should use base64url, but validation accepts both for backward compatibility
3835
const sourceFiles = ['workers/api.js', 'lib/routes-ui.js', 'lib/ui-routes/account-routes.js', 'lib/api-routes/account-routes.js'];
3936

40-
const problematicPattern = /NONCE_BYTES\)\.toString\(['"]base64['"]\)/;
41-
const violations = [];
37+
const base64urlPattern = /NONCE_BYTES\)\.toString\(['"]base64url['"]\)/;
38+
const base64Pattern = /NONCE_BYTES\)\.toString\(['"]base64['"]\)/;
39+
const usesBase64url = [];
40+
const usesBase64 = [];
4241

4342
for (const file of sourceFiles) {
4443
const filePath = path.join(__dirname, '..', file);
4544
if (fs.existsSync(filePath)) {
4645
const content = fs.readFileSync(filePath, 'utf8');
47-
const lines = content.split('\n');
48-
lines.forEach((line, index) => {
49-
if (problematicPattern.test(line)) {
50-
violations.push(`${file}:${index + 1}: ${line.trim()}`);
51-
}
52-
});
46+
if (base64urlPattern.test(content)) {
47+
usesBase64url.push(file);
48+
}
49+
if (base64Pattern.test(content)) {
50+
usesBase64.push(file);
51+
}
5352
}
5453
}
5554

56-
assert.strictEqual(violations.length, 0, `Found NONCE_BYTES with plain base64 encoding (should use base64url):\n${violations.join('\n')}`);
55+
// New nonce generation should use base64url (old base64 nonces are only accepted for backward compat)
56+
assert.strictEqual(usesBase64.length, 0, `Found NONCE_BYTES with plain base64 encoding (new code should use base64url):\n${usesBase64.join('\n')}`);
5757
});
5858

5959
await t.test('all NONCE_BYTES usages use base64url encoding', async () => {
@@ -97,12 +97,12 @@ test('OAuth nonce encoding tests', async t => {
9797
assert.strictEqual(incorrectUsages.length, 0, `Found NONCE_BYTES with incorrect encoding:\n${incorrectUsages.join('\n')}`);
9898
});
9999

100-
await t.test('nonce validation with error for invalid format', async () => {
101-
// Verify that files using data.n validate the format and throw error if invalid
102-
// This rejects old cached URLs with standard base64 nonces
100+
await t.test('nonce validation accepts both base64 and base64url formats', async () => {
101+
// Verify that files using data.n validate the format with backward-compatible regex
103102
const filesToCheck = ['lib/routes-ui.js', 'lib/ui-routes/account-routes.js'];
104103

105104
// Pattern: validates nonce and throws Boom error for invalid format
105+
// Now accepts both base64url and base64 encoding
106106
const validationPattern = /if.*!.*test\(nonce\).*\{[\s\S]*?Boom\.boomify.*Invalid nonce format/;
107107

108108
const missingValidation = [];
@@ -119,4 +119,31 @@ test('OAuth nonce encoding tests', async t => {
119119

120120
assert.strictEqual(missingValidation.length, 0, `Files using data.n must validate and reject invalid nonces:\n${missingValidation.join('\n')}`);
121121
});
122+
123+
await t.test('invalid nonces are rejected', async () => {
124+
// Test that completely invalid nonces are still rejected
125+
const invalidNonces = [
126+
'', // empty
127+
'short', // too short
128+
'a'.repeat(30), // too long (no padding scenario)
129+
'!@#$%^&*(){}[]|\\', // invalid characters
130+
'valid123456789012345===', // too much padding
131+
'valid12345678901234567890====' // way too long with padding
132+
];
133+
134+
for (const nonce of invalidNonces) {
135+
assert.ok(!NONCE_VALIDATION_REGEX.test(nonce), `Invalid nonce should be rejected: ${nonce}`);
136+
}
137+
});
138+
139+
await t.test('edge cases for base64/base64url characters', async () => {
140+
// Test specific edge cases for the character differences
141+
const base64urlOnlyChars = 'AAAAAAAAAAAAAAAAAAA_-'; // 21 chars with _ and -
142+
const base64OnlyChars = 'AAAAAAAAAAAAAAAAAAA+/=='; // 21 chars + padding with + and /
143+
const mixedChars = 'AAAAAAAAAAAAAAAA_-+/'; // mixed characters (20 chars)
144+
145+
assert.ok(NONCE_VALIDATION_REGEX.test(base64urlOnlyChars), 'base64url-only chars should pass');
146+
assert.ok(NONCE_VALIDATION_REGEX.test(base64OnlyChars), 'base64-only chars with padding should pass');
147+
assert.ok(!NONCE_VALIDATION_REGEX.test(mixedChars), 'too short nonce should fail');
148+
});
122149
});

workers/api.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2027,8 +2027,9 @@ Include your token in requests using one of these methods:
20272027
}
20282028

20292029
// Validate nonce format: 16 bytes base64url encoded = 21-22 characters
2030+
// Also accept base64 encoding (+, /, =) for backward compatibility with old cached nonces
20302031
const stateNonce = request.query.state.slice('account:add:'.length);
2031-
if (!/^[A-Za-z0-9_-]{21,22}$/.test(stateNonce)) {
2032+
if (!/^[A-Za-z0-9_\-+/]{21,22}={0,2}$/.test(stateNonce)) {
20322033
let error = Boom.boomify(new Error(`Oauth failed: invalid state format`), { statusCode: 400 });
20332034
throw error;
20342035
}

0 commit comments

Comments
 (0)