Problem/Motivation

The RecurlyPushListenerController code calls \Recurly_Account::get but doesn't pass the client object 2nd argument to the get method. And, because in this code path the client hasn't been initialized globally, the get call fails and the code throws an Recurly_UnauthorizedError error because there's no API key. And no valid client object.

Proposed resolution

Change the call from \Recurly_Account::get($notification->account->account_code); to \Recurly_Account::get($notification->account->account_code, $this->recurlyClient);

Remaining tasks

This needs tests. Probably a Kernel test that makes an HTTP POST with the XML payload the recurly would normally send to the recurly webhook listener URL, which routes to RecurlyPushListenerController. Or something like that ... Given the way this reads from file_get_content('php://input') I'm not sure how to mock the payload from recurly otherwise.

Issue fork recurly-3346810

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

eojthebrave created an issue. See original summary.

eojthebrave’s picture

Issue summary: View changes

  • eojthebrave committed 3d8fe38a on 4.x
    Issue #3346810 by eojthebrave: RecurlyPushListenerController doesn't...
eojthebrave’s picture

I added a commit to the 4.x branch that fixes the bug so that push notifications can actually be processed now. However, I also noticed there are no tests for push notifications so it would be good to add some tests too.

eojthebrave’s picture

Issue summary: View changes

eojthebrave’s picture

Status: Active » Needs review

Here's an MR that adds tests for the push notification controller. I had to make one change to the code in the controller, switching from `file_get_contents('php://input')` to `$request->getContent()` to make it easier to mock the request. And, this is probably a more appropriate use of the Drupal request API anyway. This doesn't change anything about how this works for anyone's current implementations. Just adds tests.

  • eojthebrave committed d180012b on 4.x
    Issue #3346810 by eojthebrave: RecurlyPushListenerController doesn't...
eojthebrave’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.