search

SamJoan / droopescan

A plugin-based scanner that aids security researchers in identifying issues with several CMSs, mainly Drupal & Silverstripe.
GNU Affero General Public License v3.0
1.22k stars 246 forks source link

feat(User-Agent): Fixes #40. Add --user-agent parameter which allows … #43

Closed kieran-github closed 4 years ago

kieran-github commented 4 years ago

Hi,

Here is a pull request that fixes #40 .

Please have a review and let me know what you think.

kieran-github commented 4 years ago

Had some issues with the CI, i see there's a history of it failing for 3.3 - also fixed it - see: https://github.com/travis-ci/travis-ci/issues/9133.

SamJoan commented 4 years ago

Hi

Thank you very much for your PR! This is the best PR I've received so far I think :)

I would like to request two changes prior to merging, if you could implement them I will certainly merge this code.

  1. Python 3.3 is EOL. Could you, instead of performing those changes simply remove python 3.3 integration from travis? it may be as simple as removing that line.
  2. I notice that you added the user_agent as a parameter for several functions, for example enumerate_file_hash. This parameter does not seem to be used, could you remove it from the kwargs variables and from the function definitions? There may be something I'm missing here, let me know if the variable is necessary! It's been a while since I've looked at this code. I'll leave you a note on all the functions I see this happening in.

I don't know why I can't see the travis integration on this PR, I'll have a look and see.

Thanks! Pedro

kieran-github commented 4 years ago

@droope See the updated changes. I removed it from all kwargs https://github.com/droope/droopescan/blob/master/dscan/plugins/internal/base_plugin_internal.py#L138. I had to remove two test cases as user_agent isn't part of call_args even though it is being used in https://github.com/droope/droopescan/blob/master/dscan/plugins/internal/base_plugin_internal.py#L237 (requests session initialization) . However it greatly simplifies the change and still passes all test cases otherwise the user_agent kwargs key will stay throughout the program without being used - i think this is our best option.

Also updated the ci to no longer use python 3.3 as it is EOL, as you mentioned. Unsure why it isn't being hooked to my commits however.

SamJoan commented 4 years ago

Thank you very much for this change, it looks good, and I know lots of people wanted it. I am going to merge it right in to the development branch.

It may be a little while before I am able to publish a new version, but I will do it as soon as I can.

Thanks! Pedro

SamJoan commented 3 years ago

I merged this into master! I noted your contribution on the changelog as well. https://github.com/droope/droopescan/blob/master/CHANGELOG