handle_action is called once per parameter in `base_connector.py`

[edthedev]

Context

https://github.com/splunk/pytest-splunk-soar-connectors/blob/34ef966e227328b0bc176a75040e77bd1d7ff2be/src/phantom_mock/phantom/base_connector.py#L373-L380

There appears to be a bug in def _handle_action in base_connector.py. self.__current_param is assigned, but not used within the scope of the for loop, and the for loop results in handle_action being called with the full set of parameters multiple times - once per parameter.

As far as we've noticed the actual SOAR environment only calls handle_action once with the full set of parameters.

We're thinking the loop on line 373 is just not necessary?

Example Test

Here's our test that revealed the issue:

@patch("phsoar_null_router.soar_null_router_connector.login_from_env")
def test_block(mock, connector: Soar_Null_RouterConnector):
    cidr = '157.55.39.89/32'

    mock.return_value = Mock(spec=BHRClient)
    in_json = {
            "appid": "fceeaac1-8f96-46d6-9c3b-896e363eb004",
            "parameters": {
                "identifier": "block",
                "cidr": cidr,
            }
    }

    # Execute Action
    dump = json.dumps(in_json)
    import pdb; pdb.set_trace()
    action_result_str = connector._handle_action(dump, None)
    action_result = json.loads(action_result_str)

    # Assertion
    assert action_result[0]["message"] == f"Blocked {cidr}"
    mock.return_value.block.assert_called_once()
    assert 'cidr' in mock.call_args.kwargs
    assert mock.call_args.kwargs['cidr'] == cidr

https://github.com/techservicesillinois/secops-splunk-null-router/blob/807f19e71675bfb9c41391b58d84ae757b69ba3b/tests/test_connector.py#L30-L53

On line 51 we call assert_called_once and it fails because block was called twice. Stepping through, we see it gets called once due to the identifier key, and then again due to the cidr key in self.__action_json["parameters"].

[edthedev]

@dfederschmidt Pull request #4 will address this, if we understood the issue correctly.

[dfederschmidt]

Hi @edthedev

Thanks for the comprehensive report. I think you're onto something and clearly this is not working as intended. I did some exploration on how the connector is called within the product and please find my findings below:

The _handle_action method is called once with the full configuration for the connector run. This is a method on the BaseConnector and called by SOAR internally. In this plugin, we call this as part of our unit test.

The handle_action method (no leading underscore) is implemented by your connector and is potentially called multiple times, with varying parameters. What I discovered here is that the payload format (in_json) I described in docs and used in the example implementation slightly differs from the actual payload in the product, which is what is leading to the issue you were seeing.

I reviewed #4 and while this certainly addresses the issue, it is not how the live SOAR product calls the connector at present. I'm currently in the process of revising the payload and documentation to ensure it reflects how SOAR calls the connector logic.

Thanks again for reporting this - there will be a slight change necessary to your existing test, but I'll be able to send you a MR for that. I'll also add corresponding tests to ensure this works as expected by SOAR.

[edthedev]

@dfederschmidt With things slowing down this week, I noticed we're still working from our fork on our SOAR apps.

No rush, I probably won't see your reply before I return from the holiday break. I just want to check in and see if we should move back to the main repository, or wait for this or a related change to merge.