Verified Commit 663cddd4 authored by aguestuser's avatar aguestuser
Browse files

[102] block non-admin command execution on signup channel

* this ensures that nobody can ever subscribe, so admin messages will
  never be broadcast
* that means admins can also use signup channel to talk to each other
  about fielding signups
* it also means no signup messages will fail to result in signups
  because they are mistaken for commands
parent a3dd4ae3
No preview for this file type
......@@ -29,7 +29,7 @@ const test = {
const development = {
...defaults,
signupPhoneNumber: '+13343264237',
signupPhoneNumber: process.env.SIGNUP_CHANNEL_NUMBER_DEV,
}
module.exports = {
......
......@@ -5,6 +5,9 @@ const { messagesIn } = require('./messages')
const { memberTypes } = channelRepository
const { PUBLISHER, SUBSCRIBER, NONE } = memberTypes
const { lowerCase } = require('lodash')
const {
signal: { signupPhoneNumber },
} = require('../../config')
/**
* type Executable = {
......@@ -48,8 +51,9 @@ const commands = {
******************/
// Dispatchable -> Promise<{dispatchable: Dispatchable, commandResult: CommandResult}>
const processCommand = dispatchable =>
execute(parseCommand(dispatchable.sdMessage.messageBody), dispatchable)
const processCommand = dispatchable => {
return execute(parseCommand(dispatchable.sdMessage.messageBody), dispatchable)
}
// string -> Executable
const parseCommand = msg => {
......@@ -73,6 +77,8 @@ const parseCommand = msg => {
const execute = async (executable, dispatchable) => {
const { command, payload } = executable
const { db, channel, sender } = dispatchable
// don't allow command execution on the signup channel for non-admins
if (channel.phoneNumber === signupPhoneNumber && sender.type !== PUBLISHER) return noop()
const result = await ({
[commands.ADD]: () => maybeAddPublisher(db, channel, sender, payload),
[commands.HELP]: () => maybeShowHelp(db, channel, sender),
......@@ -82,8 +88,8 @@ const execute = async (executable, dispatchable) => {
[commands.RENAME]: () => maybeRenameChannel(db, channel, sender, payload),
[commands.REMOVE]: () => maybeRemovePublisher(db, channel, sender, payload),
[commands.TOGGLE_RESPONSES]: () => maybeToggleResponses(db, channel, sender, payload),
}[command] || (() => noop(sender)))()
return { ...result, command }
}[command] || (() => noop()))()
return { command, ...result }
}
/********************
......@@ -235,11 +241,7 @@ const toggleResponses = (db, channel, newSetting, sender, cr) =>
.catch(err => logAndReturn(err, { status: statuses.ERROR, message: cr.dbError(newSetting) }))
// NOOP
const noop = sender =>
Promise.resolve({
status: statuses.NOOP,
message: messagesIn(sender.language).notifications.noop,
})
const noop = () => Promise.resolve({ command: commands.NOOP, status: statuses.NOOP, message: '' })
/**********
* HELPERS
......
......@@ -93,7 +93,6 @@ const handleBroadcastResponse = dispatchable => {
return relayBroadcastResponse(dispatchable)
}
// TODO: rename this handleCommandResult
const handleCommandResult = async ({ commandResult, dispatchable }) => {
const { message, command, status } = commandResult
await respond({ ...dispatchable, message, command, status })
......
......@@ -173,6 +173,7 @@ const verify = (sock, phoneNumber, code) =>
write(sock, { type: messageTypes.VERIFY, username: phoneNumber, code })
const awaitVerificationResult = async (sock, phoneNumber) => {
//TODO(aguestuser|2019-11-09): use signald message ids to make this await call simpler
return new Promise((resolve, reject) => {
sock.on('data', function handle(msg) {
const { type, data } = safeJsonParse(msg, reject)
......@@ -278,6 +279,7 @@ const fetchIdentities = async (sock, channelPhoneNumber, memberPhoneNumber) => {
}
const awaitIdentitiesOf = (sock, memberPhoneNumber) => {
//TODO(aguestuser|2019-11-09): use signald message ids to make this await call simpler
return new Promise((resolve, reject) => {
// create handler
const handle = msg => {
......
......@@ -16,8 +16,10 @@ import validator from '../../../../app/db/validations/phoneNumber'
import { subscriptionFactory } from '../../../support/factories/subscription'
import { genPhoneNumber } from '../../../support/factories/phoneNumber'
import { publicationFactory } from '../../../support/factories/publication'
import { messagesIn } from '../../../../app/services/dispatcher/messages'
import { sdMessageOf } from '../../../../app/services/signal'
const {
signal: { signupPhoneNumber },
} = require('../../../../app/config')
describe('executor service', () => {
describe('parsing commands', () => {
......@@ -199,6 +201,11 @@ describe('executor service', () => {
subscriptions: times(2, subscriptionFactory({ channelPhoneNumber: '+13333333333' })),
messageCount: { broadcastIn: 42 },
}
const signupChannel = {
name: 'SB_SIGNUP',
phoneNumber: signupPhoneNumber,
publications: channel.publications,
}
const publisher = {
phoneNumber: '+11111111111',
type: memberTypes.PUBLISHER,
......@@ -820,6 +827,22 @@ describe('executor service', () => {
})
})
describe('new user attempting to JOIN the signup channel', () => {
it('returns NOOP', async () => {
const dispatchable = {
db,
channel: signupChannel,
sender: randomPerson,
sdMessage: sdMessageOf(signupChannel, 'HELLO'),
}
expect(await processCommand(dispatchable)).to.eql({
command: commands.NOOP,
status: statuses.NOOP,
message: '',
})
})
})
describe('invalid command', () => {
it('returns NOOP status/message', async () => {
const dispatchable = {
......@@ -831,7 +854,7 @@ describe('executor service', () => {
expect(await processCommand(dispatchable)).to.eql({
command: commands.NOOP,
status: statuses.NOOP,
message: messagesIn('EN').notifications.noop,
message: '',
})
})
})
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment