Skip to content

Commit b027425

Browse files
committed
add async authPlugin error handlers
Errors reported by asynchronous authentication plugins are not properly handled, leading the client to become idle, at least until the server closes the connection because of inactivity. This patch introduces the changes to ensure all errors generated by asynchronous authentication plugins result in fatal 'error' events emitted at the command level (and subsequently bubbled up to the connection instance), allowing the client to immediately close the underlying socket connection and stop the process.
1 parent 5dcf1bf commit b027425

File tree

2 files changed

+109
-5
lines changed

2 files changed

+109
-5
lines changed

lib/commands/auth_switch.js

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
// This file was modified by Oracle on July 5, 2021.
2+
// Errors generated by asynchronous authentication plugins are now being
3+
// handled and subsequently emitted at the command level.
4+
// Modifications copyright (c) 2021, Oracle and/or its affiliates.
5+
16
'use strict';
27

38
const Packets = require('../packets/index.js');
@@ -17,6 +22,14 @@ function warnLegacyAuthSwitch() {
1722
);
1823
}
1924

25+
function authSwitchPluginError(error, command) {
26+
// Authentication errors are fatal
27+
error.code = 'AUTH_SWITCH_PLUGIN_ERROR';
28+
error.fatal = true;
29+
30+
command.emit('error', error);
31+
}
32+
2033
function authSwitchRequest(packet, connection, command) {
2134
const { pluginName, pluginData } = Packets.AuthSwitchRequest.fromPacket(
2235
packet
@@ -34,8 +47,7 @@ function authSwitchRequest(packet, connection, command) {
3447
warnLegacyAuthSwitch();
3548
legacySwitchHandler({ pluginName, pluginData }, (err, data) => {
3649
if (err) {
37-
connection.emit('error', err);
38-
return;
50+
return authSwitchPluginError(err, command);
3951
}
4052
connection.writePacket(new Packets.AuthSwitchResponse(data).toPacket());
4153
});
@@ -54,19 +66,20 @@ function authSwitchRequest(packet, connection, command) {
5466
if (data) {
5567
connection.writePacket(new Packets.AuthSwitchResponse(data).toPacket());
5668
}
69+
}).catch(err => {
70+
authSwitchPluginError(err, command);
5771
});
5872
}
5973

60-
function authSwitchRequestMoreData(packet, connection) {
74+
function authSwitchRequestMoreData(packet, connection, command) {
6175
const { data } = Packets.AuthSwitchRequestMoreData.fromPacket(packet);
6276

6377
if (connection.config.authSwitchHandler) {
6478
const legacySwitchHandler = connection.config.authSwitchHandler;
6579
warnLegacyAuthSwitch();
6680
legacySwitchHandler({ pluginData: data }, (err, data) => {
6781
if (err) {
68-
connection.emit('error', err);
69-
return;
82+
return authSwitchPluginError(err, command);
7083
}
7184
connection.writePacket(new Packets.AuthSwitchResponse(data).toPacket());
7285
});
@@ -82,6 +95,8 @@ function authSwitchRequestMoreData(packet, connection) {
8295
if (data) {
8396
connection.writePacket(new Packets.AuthSwitchResponse(data).toPacket());
8497
}
98+
}).catch(err => {
99+
authSwitchPluginError(err, command);
85100
});
86101
}
87102

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Copyright (c) 2021, Oracle and/or its affiliates.
2+
3+
'use strict';
4+
5+
const mysql = require('../../index.js');
6+
const Command = require('../../lib/commands/command.js');
7+
const Packets = require('../../lib/packets/index.js');
8+
9+
const assert = require('assert');
10+
11+
class TestAuthSwitchPluginError extends Command {
12+
constructor(args) {
13+
super();
14+
this.args = args;
15+
}
16+
17+
start(_, connection) {
18+
const serverHelloPacket = new Packets.Handshake({
19+
// "required" properties
20+
protocolVersion: 10,
21+
serverVersion: 'node.js rocks'
22+
});
23+
this.serverHello = serverHelloPacket;
24+
serverHelloPacket.setScrambleData(() => {
25+
connection.writePacket(serverHelloPacket.toPacket(0));
26+
});
27+
return TestAuthSwitchPluginError.prototype.sendAuthSwitchRequest;
28+
}
29+
30+
sendAuthSwitchRequest(_, connection) {
31+
const asr = new Packets.AuthSwitchRequest(this.args);
32+
connection.writePacket(asr.toPacket());
33+
return TestAuthSwitchPluginError.prototype.finish;
34+
}
35+
36+
finish(_, connection) {
37+
connection.end();
38+
return TestAuthSwitchPluginError.prototype.finish;
39+
}
40+
}
41+
42+
const server = mysql.createServer(conn => {
43+
conn.addCommand(
44+
new TestAuthSwitchPluginError({
45+
pluginName: 'auth_test_plugin',
46+
pluginData: Buffer.allocUnsafe(0)
47+
})
48+
);
49+
});
50+
51+
let error;
52+
let uncaughtExceptions = 0;
53+
54+
const portfinder = require('portfinder');
55+
portfinder.getPort((_, port) => {
56+
server.listen(port);
57+
const conn = mysql.createConnection({
58+
port: port,
59+
authPlugins: {
60+
auth_test_plugin () {
61+
return function () {
62+
return Promise.reject(Error('boom'));
63+
}
64+
}
65+
}
66+
});
67+
68+
conn.on('error', err => {
69+
error = err;
70+
71+
conn.end();
72+
server.close();
73+
});
74+
});
75+
76+
process.on('uncaughtException', err => {
77+
// The plugin reports a fatal error
78+
assert.equal(error.code, 'AUTH_SWITCH_PLUGIN_ERROR');
79+
assert.equal(error.message, 'boom');
80+
assert.equal(error.fatal, true);
81+
// The server must close the connection
82+
assert.equal(err.code, 'PROTOCOL_CONNECTION_LOST');
83+
84+
uncaughtExceptions += 1;
85+
});
86+
87+
process.on('exit', () => {
88+
assert.equal(uncaughtExceptions, 1);
89+
});

0 commit comments

Comments
 (0)