Talk: Bug: Akismet plugin failing

[icco]

Do you want to request a feature or report a bug?

Bug!

Intended outcome:

We were trying to implement the talk akismet plugin. We should see reports of comments sent to Akismet.

Actual outcome:

So in prod, running 4.4.0, we weren't seeing any comments in the akismet dashboard. I reached out to the Akismet team, and they said they weren't seeing any requests land for our domain or API key. I filed (and you released :D ) #1622 after further discussion with the Akismet team said that:

Yes, indeed. Using the is_test parameter set to true, or 1, would do exactly what you described. The intent of that parameter is for things such as unit tests, so that you can get consistent results for such tests, by essentially turning off all of the machine learning aspects of our API, logging, and so on.

So, I went to dev and upgraded our Talk to 4.4.1, which also has DEBUG=* set in it's environment. That's when I saw the log line:

2018-05-15T19:58:17.994Z talk:plugin:akismet no ip on request

This seemed weird to me, so I forked your plugin and discovered that the req variable from https://github.com/coralproject/talk/blob/v4.4.1/plugins/talk-plugin-akismet/server/hooks.js#L31 seems to never have the following properties:

• Get()
• ip
• headers

I tried lots of different ways to get the data we needed out of req but was unsuccessful. One example diff I tried was

diff --git a/plugins/flm-akismet/server/hooks.js b/plugins/flm-akismet/server/hooks.js
index 025789a..5c0bd21 100644
--- a/plugins/flm-akismet/server/hooks.js
+++ b/plugins/flm-akismet/server/hooks.js
@@ -37,12 +37,11 @@ module.exports = {

         let spam = false;
         try {
-          debug(req)
-          const user_ip = get(req, 'ip', '');
+          const user_ip = get(req, 'ip', '') || req.headers['x-forwarded-for'];

           // Get some headers from the request.
-          const user_agent = req.get('User-Agent') || '';
-          const referrer = req.get('Referrer') || '';
+          const user_agent = req.headers['user-agent'];
+          const referrer = req.headers['referer'];

           // Get the Asset that the comment is being made against.
           const asset = await loaders.Assets.getByID.load(input.asset_id);
@@ -52,7 +51,7 @@ module.exports = {
           }

           // Send off the comment to Akismet to check to see what they say.
-          spam = await client.checkSpam({
+          let commentData = {
             user_ip,
             user_agent,
             referrer,
@@ -60,7 +59,9 @@ module.exports = {
             comment_type: 'comment',
             comment_content: input.body,
             is_test: false,
-          });
+          };
+          debug(commentData);
+          spam = await client.checkSpam(commentData);

           debug(`comment analyzed as ${spam ? 'being' : 'not being'} spam`);
         } catch (err) {

Which means this plugin will never send data to akismet as according to https://akismet.com/development/api/#comment-check the following values must exist:

 - user_ip  
 - user_agent   
 - referrer

Dumping out the req object revealed a large object that did have the data in there, but I could not figure out how to get the data out.

One way to test this might be to remove checks in the plugin and instead catch the errors thrown by the library: https://github.com/chrisfosterelli/akismet-api/blob/master/lib/akismet.js#L85

Things to note that are smaller bugs:

• The header you want is referer not referrer, which is just a typo
• It might be useful to get the IP from the x-forwarded-for header, as I did not see the request IP anywhere else in the object
• When https://github.com/chrisfosterelli/akismet-api/pull/80 is merged and released, you'll want to add support to talk for better insight into comment spam

How to reproduce the issue:

Set DEBUG=talk:plugin:* and the correct variables for the talk akismet plugin. We tried this with both a Pro and a Free akismet account with the same results as the bug is in the code and not with the dependency.

Version and environment

We are running in Docker + ECS. Versions listed above.

[icco]

Note, I looked around at all the other Talk plugins and couldn't find another that was a pre hook that accessed request data, nor could I really figure out how that data would get there. I would love to know more about how to write plugins like this that have access to request data in the https://docs.coralproject.net/talk/plugin-recipes/ docs

[kgardnr]

@icco If you wanna check Toxic Comments that would be a somewhat similar type plugin to Akismet with pre hooks: https://github.com/coralproject/talk/tree/master/plugins/talk-plugin-toxic-comments/server

We've got this logged as a bug too and will take a look

[icco]

Thanks @kgardnr. From what I could tell, that plugin doesn't get request data at all, and it's func signature is different, which is why I avoided it.

[wyattjoh]

Fixed in #1638