Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(websocket): Adding support for redis username in websocket server #25826

Merged
merged 3 commits into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions superset-websocket/config.test.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
{
"redis": {
"port": 6379,
"host": "127.0.0.1",
"password": "",
"db": 10,
"ssl": false
"password": "some pwd"
},
"statsd": {
"host": "127.0.0.1",
Expand Down
46 changes: 28 additions & 18 deletions superset-websocket/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions superset-websocket/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
},
"license": "Apache-2.0",
"dependencies": {
"@types/lodash": "^4.14.200",
"cookie": "^0.5.0",
"hot-shots": "^10.0.0",
"ioredis": "^4.28.0",
"jsonwebtoken": "^9.0.2",
"lodash": "^4.17.21",
"uuid": "^9.0.1",
"winston": "^3.11.0",
"ws": "^8.14.2"
Expand Down
10 changes: 9 additions & 1 deletion superset-websocket/spec/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ test('buildConfig() builds configuration and applies env var overrides', () => {
);
expect(config.redis.host).toEqual('127.0.0.1');
expect(config.redis.port).toEqual(6379);
expect(config.redis.password).toEqual('');
expect(config.redis.password).toEqual('some pwd');
expect(config.redis.db).toEqual(10);
expect(config.redis.ssl).toEqual(false);
expect(config.statsd.host).toEqual('127.0.0.1');
Expand Down Expand Up @@ -65,3 +65,11 @@ test('buildConfig() builds configuration and applies env var overrides', () => {
delete process.env.STATSD_PORT;
delete process.env.STATSD_GLOBAL_TAGS;
});

test('buildConfig() performs deep merge between configs', () => {
const config = buildConfig();
// We left the ssl setting the default
expect(config.redis.ssl).toEqual(false);
// We overrode the pwd
expect(config.redis.password).toEqual('some pwd');
});
29 changes: 23 additions & 6 deletions superset-websocket/spec/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,36 +138,53 @@ describe('server', () => {
describe('redisUrlFromConfig', () => {
test('it builds a valid Redis URL from defaults', () => {
expect(
server.redisUrlFromConfig({
server.buildRedisOpts({
port: 6379,
host: '127.0.0.1',
username: 'test-user',
password: '',
db: 0,
ssl: false,
validateHostname: false,
}),
).toEqual('redis://127.0.0.1:6379/0');
).toEqual({ db: 0, host: '127.0.0.1', port: 6379 });
});
test('it builds a valid Redis URL with a password', () => {
expect(
server.redisUrlFromConfig({
server.buildRedisOpts({
port: 6380,
host: 'redis.local',
username: 'cool-user',
password: 'foo',
db: 1,
ssl: false,
validateHostname: false,
}),
).toEqual('redis://:foo@redis.local:6380/1');
).toEqual({
db: 1,
host: 'redis.local',
password: 'foo',
port: 6380,
username: 'cool-user',
});
});
test('it builds a valid Redis URL with SSL', () => {
expect(
server.redisUrlFromConfig({
server.buildRedisOpts({
port: 6379,
host: '127.0.0.1',
password: '',
username: 'cool-user',
db: 0,
ssl: true,
validateHostname: false,
}),
).toEqual('rediss://127.0.0.1:6379/0');
).toEqual({
db: 0,
host: '127.0.0.1',
port: 6379,
tls: { checkServerIdentity: expect.anything() },
});
});
});

Expand Down
26 changes: 18 additions & 8 deletions superset-websocket/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,19 @@
* specific language governing permissions and limitations
* under the License.
*/

import * as _ from 'lodash';

export interface RedisConfig {
port: number;
host: string;
password: string;
username: string;
db: number;
ssl: boolean;
validateHostname: boolean;
}

type ConfigType = {
port: number;
logLevel: string;
Expand All @@ -26,13 +39,7 @@ type ConfigType = {
port: number;
globalTags: Array<string>;
};
redis: {
port: number;
host: string;
password: string;
db: number;
ssl: boolean;
};
redis: RedisConfig;
redisStreamPrefix: string;
redisStreamReadCount: number;
redisStreamReadBlockMs: number;
Expand Down Expand Up @@ -70,8 +77,10 @@ function defaultConfig(): ConfigType {
host: '127.0.0.1',
port: 6379,
password: '',
username: 'default',
db: 0,
ssl: false,
validateHostname: true,
},
};
}
Expand Down Expand Up @@ -114,6 +123,7 @@ function applyEnvOverrides(config: ConfigType): ConfigType {
REDIS_HOST: val => (config.redis.host = val),
REDIS_PORT: val => (config.redis.port = toNumber(val)),
REDIS_PASSWORD: val => (config.redis.password = val),
REDIS_USERNAME: val => (config.redis.username = val),
REDIS_DB: val => (config.redis.db = toNumber(val)),
REDIS_SSL: val => (config.redis.ssl = toBoolean(val)),
STATSD_HOST: val => (config.statsd.host = val),
Expand All @@ -132,6 +142,6 @@ function applyEnvOverrides(config: ConfigType): ConfigType {
}

export function buildConfig(): ConfigType {
const config = Object.assign(defaultConfig(), configFromFile());
const config = _.merge(defaultConfig(), configFromFile());
return applyEnvOverrides(config);
}
48 changes: 33 additions & 15 deletions superset-websocket/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ import WebSocket from 'ws';
import { v4 as uuidv4 } from 'uuid';
import jwt, { Algorithm } from 'jsonwebtoken';
import cookie from 'cookie';
import Redis from 'ioredis';
import Redis, { RedisOptions } from 'ioredis';
import StatsD from 'hot-shots';

import { createLogger } from './logger';
import { buildConfig } from './config';
import { buildConfig, RedisConfig } from './config';
import { checkServerIdentity, PeerCertificate } from 'tls';

export type StreamResult = [
recordId: string,
Expand Down Expand Up @@ -66,13 +67,6 @@ export interface SocketInstance {
channel: string;
pongTs: number;
}
interface RedisConfig {
port: number;
host: string;
password?: string | null;
db: number;
ssl: boolean;
}

interface ChannelValue {
sockets: Array<string>;
Expand Down Expand Up @@ -103,15 +97,39 @@ export const statsd = new StatsD({
if (startServer && opts.jwtSecret.length < 32)
throw new Error('Please provide a JWT secret at least 32 bytes long');

export const redisUrlFromConfig = (redisConfig: RedisConfig): string => {
let url = redisConfig.ssl ? 'rediss://' : 'redis://';
if (redisConfig.password) url += `:${redisConfig.password}@`;
url += `${redisConfig.host}:${redisConfig.port}/${redisConfig.db}`;
return url;
export const buildRedisOpts = (baseConfig: RedisConfig) => {
const redisOpts: RedisOptions = {
port: baseConfig.port,
host: baseConfig.host,
db: baseConfig.db,
};

const passwd = baseConfig.password;
if (passwd !== '') {
redisOpts.username = baseConfig.username;
redisOpts.password = baseConfig.password;
}

if (baseConfig.ssl) {
redisOpts.tls = {
checkServerIdentity: (
hostname: string,
cert: PeerCertificate,
): Error | undefined => {
// Note, the cert chain will have been verified already. the role of this method is to
// validate that at least one of the SAN's (or subject) of the server's cert matches the provided hostname
if (baseConfig.validateHostname) {
return checkServerIdentity(hostname, cert);
}
},
};
}

return redisOpts;
};

// initialize servers
const redis = new Redis(redisUrlFromConfig(opts.redis));
const redis = new Redis(buildRedisOpts(opts.redis));
const httpServer = http.createServer();
export const wss = new WebSocket.Server({
noServer: true,
Expand Down
Loading