search

mozilla / testdaybot

Mozilla QA Test Day IRC Bot
9 stars 12 forks source link

add user verification for each admin command #28

Closed galgeek closed 9 years ago

galgeek commented 9 years ago

probably best to write a function to check for admin nick and logged in user. client.whois crashes the bot if the supplied nick is not currently available on IRC updates to the irc library are available and may address this crash, but adding test to individual admin commands on execution would be more secure and less likely to cause the crash PR #26

whimboo commented 9 years ago

Actually it does not crash the bot. It's a simple exception you would like to handle in our code for the time being. Please also add the traceback so it's known what this issue is about.

galgeek commented 9 years ago

Here's the traceback:

{ prefix: 'belew.mozilla.org', server: 'belew.mozilla.org', command: 'err_nosuchnick', rawCommand: '401', commandType: 'error', args: [ '_TestDayBot', 'AutomatedTester', 'No such nick/channel' ] }

/Users/bara/Dev/testdaybot/node_modules/irc/lib/irc.js:520 throw err; ^ TypeError: Cannot read property 'nick' of undefined at Client.callbackWrapper (/Users/bara/Dev/testdaybot/node_modules/irc/lib/irc.js:666:22) at Client.emit (events.js:95:17) at Client. (/Users/bara/Dev/testdaybot/node_modules/irc/lib/irc.js:253:22) at Client.emit (events.js:95:17) at /Users/bara/Dev/testdaybot/node_modules/irc/lib/irc.js:517:22 at Array.forEach (native) at Socket. (/Users/bara/Dev/testdaybot/node_modules/irc/lib/irc.js:514:15) at Socket.emit (events.js:95:17) at Socket. (_stream_readable.js:764:14) at Socket.emit (events.js:92:17)

whimboo commented 9 years ago

Marking as P1 given that we have to get this fixed for the next release.

whimboo commented 9 years ago

Just to add (and make it clearer)... when an admin command is processed we should check if the user issuing the command is actually an admin, and also has a registered nick! Otherwise other people could spoof the nick.

galgeek commented 9 years ago

pull request submitted

https://github.com/mozilla/testdaybot/pull/33

whimboo commented 9 years ago

PR #33 has been merged.