-
Notifications
You must be signed in to change notification settings - Fork 468
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Turning OWTF into a Python package #875
Conversation
@viyatb Very happy to see such changes :) I won't be able to review anything until starting next week though. I hope you won't mind :s Even then it might take time to review so many changes, although most are simply moving things around. I'm surprised regarding your comment about the PITA of including non-python files. I thought that it was fairly easy but I'm surely remembering wrong. I'll have to check next week see what I can find. |
@DePierre no problem, I'll merge it after fixing the tests. This goes on develop branch, so it is not a big deal (the branch is by default unstable and should have breaking changes). |
@viyatb |
@saganshul re gitmodule: We discussed this on Slack, we'll have to move it to the installation part (download the templates into |
@viyatb re gitmodule: yeah! we can do that. So basically, if we have plans to render everything using ReactJS so there are two ways of working in webpack environment. ( I will remove the gulp completely because webpack will be able to handle development things.
So moving webui/src/ directory actually make sense. IMO if user is installing/building python module, installer should not build any of JS module. Let me know your thoughts on the same. |
@saganshul You are right. The only problem there is we would have to add a hack to get the path before the owtf package is installed and add that to the UI config so that it builds the |
@viyatb tell me one thing, after this PR how will user launch the UI? |
@saganshul a normal run is sufficient, This is the first step of many. I would like to have different commands to run OWTF as the user wishes, say for example, to only run CLI, the user would have to run |
@viyatb One thing we can do we can host bundle.js and bundle.css somewhere and wget them at the time of installation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of remarks.
owtf/db/transaction_manager.py
Outdated
@@ -170,6 +170,7 @@ def get_first(self, criteria, target_id=None): | |||
:return: | |||
:rtype: | |||
""" | |||
print criteria, target_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like debug output to me.
owtf/db/transaction_manager.py
Outdated
@@ -195,7 +196,7 @@ def get_transaction(self, trans): | |||
:return: | |||
:rtype: | |||
""" | |||
if trans: | |||
if (trans and len(trans) > 0): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parenthesis are superfluous.
owtf/db/transaction_manager.py
Outdated
@@ -217,7 +218,9 @@ def get_transactions(self, transactions): | |||
""" | |||
owtf_tlist = [] | |||
for transaction_obj in transactions: | |||
owtf_tlist.append(self.get_transaction(transaction_obj)) | |||
print self.get_transaction(transaction_obj) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like debug output to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is.
owtf/db/transaction_manager.py
Outdated
@@ -217,7 +218,9 @@ def get_transactions(self, transactions): | |||
""" | |||
owtf_tlist = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be possible to shorten those lines with:
return [self.get_transaction(transaction) for transaction in transactions if self.get_transaction(transaction) is not None]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
owtf/db/url_manager.py
Outdated
|
||
def add_urls_end(self): | ||
num_urls_after = self.get_num_urls() | ||
msg = str(num_urls_after - self.num_urls_before)+" URLs have been added and classified" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space surrounding the +
operator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the function.
owtf/db/url_manager.py
Outdated
def add_urls_end(self): | ||
num_urls_after = self.get_num_urls() | ||
msg = str(num_urls_after - self.num_urls_before)+" URLs have been added and classified" | ||
logging.info(msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this debug output?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not, but the function is not used anyway. I am removing it.
owtf/db/url_manager.py
Outdated
count = self.db.session.query(models.Url(target_id=target_id)).count() | ||
return count | ||
|
||
def add_urls_start(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the name of the function accurate? I am not sure to understand its purpose when I read add_urls_start
and I see that it stores the number of URLs to a class attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ported it from a commit while back because the plugin helper had a method which was calling it. I removed it from there.
owtf/http/requester.py
Outdated
except HTTPError as Error: # page NOT found. | ||
# Error is really a response for anything other than 200 OK in urllib2 :) | ||
self.http_transaction.set_transaction(False, raw_request[0], Error) | ||
except urlError as Error: # Connection refused? | ||
except URLError as Error: # Connection refused? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error
should be snake case and not camel case.
owtf/http/transaction.py
Outdated
@@ -17,6 +17,7 @@ | |||
except ImportError: | |||
from httplib import responses as response_messages | |||
|
|||
from owtf.lib.general import * |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wildcard import should be avoided at all cost to ease debugging and prevent scope override.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
owtf/plugin/plugin_handler.py
Outdated
@@ -552,7 +552,7 @@ def process_plugins_for_target_list(self, plugin_group, status, target_list): | |||
{'SomeAborted': False, 'SomeSuccessful': False, 'AllSkipped': True}, | |||
{target}) | |||
else: | |||
pass | |||
exit(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure about the context. Is the user expecting OWTF to exit in such case? Is it an actual success?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed, was just for debugging. Removing this.
@DePierre @saganshul @7a ready to merge? :) |
This PR is the first of many steps to refactoring OWTF and make it installable as a Python package.
While most of the changes are self-explanatory, here are a couple of notes on the work:
MANIFEST.in
file to correct that.configuration
toconf
to separate it from the other config folder.python setup.py install
. I removed the virtualenv setup completely, since now it is the user's job to runpython setup.py install
in a separate virtualenv for maximum compatibility.~/.owtf
.Steps for installation
(I have not yet uploaded the package to PyPi, yet!)
virtualenv <env>
and activate the environment.python setup.py install
which install OWTF as a package and starts the postsetup install script.python -m owtf
.After this, OWTF should no longer be responsible for
Let me know if there are any questions.