search

gophish / api-client-python

A Python API Client for Gophish
MIT License
45 stars 48 forks source link

Creating a new Template without Attachments fails #13

Closed cschwartz closed 5 years ago

cschwartz commented 5 years ago

Hi, when playing with the API I encountered the following issue: When posting a new template without attachments to a GoPhish 7.1 instance, the following code taken from the example

template = Template(name='Test Template', html='<html><body>Click <a href="{{.URL}}">here</a></body></html>')
template = api.templates.post(template)

fails with the message *** TypeError: 'NoneType' object is not iterable.

Replacing the definition of template with the somewhat hacky

template = Template(name='Test Template',html='<html><body>Click <a href="{{.URL}}">here</a></body></html>', attachments=[Attachment.parse({"name":"a", "type":"a", "content":"a"})])

does not trigger the error.

Upon digging into it, I found that GoPhish returns the following JSON as a response to the POST above:

{
  "id": 18,
  "name": "mail_1",
  "subject": "",
  "text": "",
  "html": "test",
  "modified_date": "2018-11-01T21:00:10.314402993Z",
  "attachments": null
}

which fails to deserialize in Template.parse, due to attachments being null.

A quick fix would be changing the line above to

elif key == 'attachments' and val:

, alternatively GoPhish itself could return an empty array instead of null.

I'm not sure what GoPhish's behaviour is in similar cases and thus what the appropriate fix would be.

If you got a suggestion, I'd love to provide a pull request to resolve this.

Best, Christian

jordan-wright commented 5 years ago

Hey @cschwartz,

Thanks for reaching out! That's a great find. I guess I just assumed the list comprehension there would default to empty over the None value, but sure enough that's not happening.

I think your proposed fix would be just fine if you'd like to send up a pull request.

Thanks again for the detailed analysis! I love seeing issues like these 😄

cschwartz commented 5 years ago

Hey @jordan-wright,

thanks for the quick reply. I've prepared a pull request #14 to address this.

Best, Christian