akhoury / nodebb-plugin-spam-be-gone

yup
MIT License
23 stars 25 forks source link

Problem executing hook: filter:register.check err: undefined #22

Closed a5mith closed 9 years ago

a5mith commented 9 years ago

Don't get any other info than that every time a user is denied registration. Not sure if that hooks kaput. :thumbsup:

{"hook": "filter:register.check", "method": "checkRegister", "priority": 5},

akhoury commented 9 years ago

I need:

barisusakli commented 9 years ago

For reference that hooks is now

plugins.fireHook('filter:register.check', {req: req, res: res, userData: userData}, function(err, data) {

And this plugin is already updated AFAIK.

a5mith commented 9 years ago

Oki doke. Herm, 0.6.0, git rev-parse HEAD = 604dac88e3095b1ea130fe79fab97616d056206d

spam-be-gone version is currently 0.3.55

Honeypot only. I did update yesterday as I was updating to master and updating plugins etc. The plugin still works, it just gives that error after each time a spammer was denied reg.

akhoury commented 9 years ago

@a5mith

it just gives that error after each time a spammer was denied reg.

how do you know that?

a5mith commented 9 years ago

Logs. :laughing: Note the timestamps. (I have no qualms about publishing spammers email addresses. :D

2014-11-12T12:57:12.062Z - warn: [plugins/spam-be-gone] skiliabluelia | produceuwry@gmail.com was detected as spammer and was denied registration.
2014-11-12T12:57:12.063Z - error: [plugins] Problem executing hook: filter:register.check err: undefined
2014-11-12T13:04:20.650Z - warn: [plugins/spam-be-gone] Prailleme | penchantawmc@gmail.com was detected as spammer and was denied registration.
2014-11-12T13:04:20.651Z - error: [plugins] Problem executing hook: filter:register.check err: undefined
2014-11-12T13:04:21.523Z - warn: [plugins/spam-be-gone] Prailleme | penchantawmc@gmail.com was detected as spammer and was denied registration.
2014-11-12T13:04:21.523Z - error: [plugins] Problem executing hook: filter:register.check err: undefined
2014-11-12T13:10:19.239Z - warn: [plugins/spam-be-gone] ovessyAtrorgo | ornamentaljiym@gmail.com was detected as spammer and was denied registration.
2014-11-12T13:10:19.241Z - error: [plugins] Problem executing hook: filter:register.check err: undefined
2014-11-12T13:10:20.060Z - warn: [plugins/spam-be-gone] ovessyAtrorgo | ornamentaljiym@gmail.com was detected as spammer and was denied registration.
2014-11-12T13:10:20.061Z - error: [plugins] Problem executing hook: filter:register.check err: undefined
2014-11-12T13:16:26.944Z - warn: [plugins/spam-be-gone] ignonforA | heightenfiea@gmail.com was detected as spammer and was denied registration.
2014-11-12T13:16:26.945Z - error: [plugins] Problem executing hook: filter:register.check err: undefined
2014-11-12T13:16:27.787Z - warn: [plugins/spam-be-gone] ignonforA | heightenfiea@gmail.com was detected as spammer and was denied registration.
2014-11-12T13:16:27.788Z - error: [plugins] Problem executing hook: filter:register.check err: undefined
akhoury commented 9 years ago

can you add a log statement for me right before https://github.com/akhoury/nodebb-plugin-spam-be-gone/blob/master/library.js#L157

Plugin.checkRegister = function(data, callback) {
    async.parallel([
        function(next) {
            Plugin._honeypotCheck(data.req, data.res, data.userData, next);
        },
        function(next) {
            Plugin._recaptchaCheck(data.req, data.res, data.userData, next)
        }
    ], function(err, results) {
       console.log("checkRegister", err, results, typeof callback, recaptchaArgs == null);  // <-- add this line
       callback(err, data);
    });
};
akhoury commented 9 years ago

and then maybe use a combination of username/email to mimic a spammer attempt registration

a5mith commented 9 years ago

I was unable to stop myself signing up using spam emails and usernames, as my IP Address is different. I will need to wait until I get a spam user to sign up. However, the output I get is (with some info redacted) Unless you have a test account built in that will always trigger? :laughing:

 warn: [plugins/spam-be-gone] username:ignonforA ip:90.xxx.xx.xxx was not found in Honeypot database
checkRegister undefined [ { email: 'heightenfiea@gmail.com',
username: 'ignonforA',
password: 'apassword',
'password-confirm': 'apassword',
'agree-terms': 'on',
_csrf: 'ESGKa2jt-oiJOHSzIkcgN9GbYqqz3yo5a1co',
referrer: 'http://35hz.co.uk/' },
  { email: 'heightenfiea@gmail.com',
username: 'ignonforA',
password: 'apassword',
'password-confirm': 'apassword',
'agree-terms': 'on',
_csrf: 'ESGKa2jt-oiJOHSzIkcgN9GbYqqz3yo5a1co',
referrer: 'http://35hz.co.uk/' } ] function true
akhoury commented 9 years ago

hmm something does not make sense, based on the "denial-logs", this error should have occurred here - but I cannot be sure. i need to see that extra log when you get another spam attempt.

a5mith commented 9 years ago
2014-11-13T04:23:15.991Z - error: [plugins] Problem executing hook: filter:register.check err: undefined
2014-11-13T04:23:16.335Z - warn: [plugins/spam-be-gone] xvcweybqmoerznl | vbnvbnvbnvbnv@hotmail.com was detected as spammer and was denied registration.
checkRegister { source: 'honeypot',
message: 'xvcweybqmoerznl | vbnvbnvbnvbnv@hotmail.com was detected as spammer' } [ { email: 'vbnvbnvbnvbnv@hotmail.com',
username: 'xvcweybqmoerznl',
password: 'sdiEE2ov3',
'password-confirm': 'sdiEE2ov3',
'agree-terms': 'on',
submit: '',
_csrf: 'Ifr1ig7t-4Kpo5c4GNK8vNd9qwTlhLPikhv4',
referrer: '' },
 { email: 'vbnvbnvbnvbnv@hotmail.com',
username: 'xvcweybqmoerznl',
password: 'sdiEE2ov3',
'password-confirm': 'sdiEE2ov3',
'agree-terms': 'on',
submit: '',
_csrf: 'Ifr1ig7t-4Kpo5c4GNK8vNd9qwTlhLPikhv4',
referrer: '' } ] function true
2014-11-13T04:23:16.338Z - error: [plugins] Problem executing hook: filter:register.check err: undefined
akhoury commented 9 years ago

ok - I shouldve just done that from the beginning.

so In NodeBB core, if you look at the this register function - the filter:register.check is expected to callback(err) to stop the use from registering,

Not look at this fireFilterHook() - any filter hook that callback(err) - it is assumed that a problem happened and that err has a stack - but the errors that filter:register.check do not have a stack trace.

In anyways, the error message that you are seeing is harmless, but still, it is annoying, so what is the best way to fix it? not sure.

@barisusakli ideas?

barisusakli commented 9 years ago

I will remove that winston log in fireFilterHook, but I think you should change these to actual error objects.

next({source: 'honeypot', message: message}, userData); to something like

next(new Error('[[error:registration-denied]]', userData); or something to that effect.

Same for the recaptcha one and other if there are any.

akhoury commented 9 years ago

hmm - ok, just don't remove that log then. I will change mine.

akhoury commented 9 years ago

wait, even if I used Error objects to prevent registration, this hook would still log the error. https://github.com/NodeBB/NodeBB/blob/master/src/plugins.js#L427

barisusakli commented 9 years ago

Yeah thats what I mean with

I will remove that winston log in fireFilterHook, but I think you should change these to actual error objects.

Even then the error will bubble up all the way to the express error handler and get logged :)

akhoury commented 9 years ago

so what do? I already changed to use Error object in SPG@0.4.0

barisusakli commented 9 years ago

Actually never mind if I remove the log inside fireFilterHook you won't get any logs it will just send this back to the user.

https://github.com/NodeBB/NodeBB/blob/master/src/routes/authentication.js#L248-L250

What is the actual problem here? We don't want to get a log for every denied registration?

akhoury commented 9 years ago

in order to prevent registration I have to hook-callback with error, so yea it's logging that there was a problem. I think the error log should just not say problem. Log the error message as it is if it finds one.

barisusakli commented 9 years ago

https://github.com/NodeBB/NodeBB/commit/64cc0f244d6e14386813d34842a93e768566b041