pipenv enviroment #325

wolfthefallen · 2018-10-08T21:43:30Z

This PR creates a python virtual environment to run King Phisher in.

This is done by utilizing pipenv. This python library makes it easy for configuration of both a production, development environments. As will as supporting different versions of python all contained within their own environment. Making this change allows easier development on the King Phisher project while making a more ease of use for end users, as it does not effect any python packages that may have been installed with their package manager, or breaking other python programs by changing system package versions.

tools/install.sh will now install all required system packages and then call ./KingPhisher to set up the pipenv environment.

In addition to this, when starting King Phisher's Client or Server if the virtual environment is not found it will set it up prior to launching the application.

As there is no longer a requirements.txt file, upgrading is now handled by ./KingPhisher or ./KingPhisherServer with the switch --env_update. Which will call pipenv to sync, recreate the Pipefile.lock and reinstall the virtual environment.

Please Note at the time of this PR their is an issue with pipenv 2018.7.1 and pip 18.1. The issue has been resolved with 2018.7.1.dev0, but is not released with pypi. The installation script and pipenv wrapper for the client and server will install pipenv directly from github as a work around. Once the next version of pipenv is released to pypi this should be revisited.

To support having a wrapper for King Phisher's applications, a new utilities file has been created called clean_utilities.py this file should only contain functions and or classes that does only utilizes native python libraries. As this is imported for key functionality of the wrapper prior to the virtual environment being set up.

testing:

tools/install.sh installs King Phisher on supported operating systems (I have tested it on Ubuntu 18, Fedora, and Kali rolling)
'./KingPhisherand/KingPhisherServerwill check for the virtual enviroment and if not callpipenv` to install it.
'./KingPhisherand/KingPhisherServer --env_updateand--env_install` update/install the pipenv environment and cleanly exits.

zeroSteiner

There's also alot of partial commits here from the workflow process. When you've made these changes can you please use git squash to condense them down into a single commit? If there are more changes we can build on that one base commit instead of 10 / 11.

zeroSteiner · 2018-10-09T01:27:10Z

.gitignore

@@ -24,6 +24,7 @@ yacctab.py
 DODO
 todo.txt
 notes.txt
+Pipfile.lock


This file shouldn't be ignored, it specifies the versions we should be using. It's recommended by the author of pipenv.

zeroSteiner · 2018-10-09T01:27:32Z

KingPhisher

@@ -1,7 +1,7 @@
 #!/usr/bin/python3 -B
 # -*- coding: utf-8 -*-
 #
-#  KingPhisher
+#  KingPhisherVENV.py


This is wrong.

zeroSteiner · 2018-10-09T01:27:50Z

KingPhisher

@@ -34,74 +34,59 @@ import argparse
 import logging
 import os
 import sys
-import threading
-import time
+import subprocess


subprocess should be before sys.

zeroSteiner · 2018-10-09T01:28:52Z

KingPhisher

-	parser.add_argument('--no-plugins', dest='use_plugins', default=True, action='store_false', help='disable all plugins')
-	parser.add_argument('--no-style', dest='use_style', default=True, action='store_false', help='disable interface styling')
-	arguments = parser.parse_args()
+	parser = argparse.ArgumentParser(description='King Phisher pipenv wrapper', conflict_handler='resolve')


The description should still be King Phisher Client GUI.

zeroSteiner · 2018-10-09T01:29:48Z

KingPhisher

 	if sys.version_info < (3, 4):
-		color.print_error('the Python version is too old (minimum required is 3.4)')
+		print('the Python version is too old (minimum required is 3.4)')


Still add in the [-] prefix that the color.print_error would add.

zeroSteiner · 2018-10-09T01:41:19Z

king_phisher/clean_utilities.py

+	return parser
+
+def argp_add_wrapper(parser):
+	parser.add_argument('--env_update', dest='pipenv_update', default=False, action='store_true', help='update pipenv requirments and exit')


Please put these into a group, and use dashes in the names not underscores for consistency. You also misspelled requirements.

zeroSteiner · 2018-10-09T01:42:11Z

king_phisher/client/__main__.py

@@ -0,0 +1,106 @@
+#!/usr/bin/python3 -B


This should be removed since this file is no longer executable. Correct these first 4 lines to look like the other files, including the file name.

zeroSteiner · 2018-10-09T05:29:48Z

king_phisher/server/__main__.py

@@ -0,0 +1,244 @@
+#!/usr/bin/python3 -B


Same here, these first 4 lines should be updated.

zeroSteiner · 2018-10-09T05:30:56Z

king_phisher/clean_utilities.py

+		return os.path.abspath(program)
+	return None
+
+def argp_add_defualt_args(parser, default_root=''):


You misspelled "default" in the function name, but oddly enough it's spelled correctly in the keyword argument.

zeroSteiner · 2018-10-09T05:31:54Z

tools/development/pipenv_wrapper

@@ -0,0 +1,138 @@
+#!/usr/bin/python3 -B


I don't see where this file is referenced at all or why it is necessary. It seems redundant.

zeroSteiner · 2018-10-09T23:39:26Z

king_phisher/clean_utilities.py

+
+def argp_add_wrapper(parser):
+	parser.add_argument('--env_update', dest='pipenv_update', default=False, action='store_true', help='update pipenv requirments and exit')
+	parser.add_argument('--env_install', dest='pipenv_install', default=False, action='store_true', help='install pipenv enviroment and exit')


The word environment is misspelled here.

Pipenv install updates Set entries as the wrapper Updated basemaps to v1.2.0 pipenv pip18 fix add clean utilities Install pipenv from github Added gobject-interspection for package installs Pipenv enviroment

zeroSteiner

Please also remove tools/development/pipenv_wrapper.

zeroSteiner · 2018-10-16T13:22:15Z

KingPhisher

+		logger.info('installing pipenv')
+		process_output, return_code = startup.run_process([sys.executable, '-m', 'pip', 'install', 'pipenv'], cwd=target_directory)
+		if return_code:
+			logger.warning("the following issue occurred during installation of pipenv: {}".format(process_output))


Since we're not continuing here, this should be an error and you should be returning non-zero. I think returning return_code would actually be a good idea so the error status "bubbles up".

zeroSteiner · 2018-10-16T13:22:43Z

KingPhisher

-	arguments = parser.parse_args()
+	startup.argp_add_wrapper(parser)
+	startup.argp_add_client(parser)
+	startup.argb_add_default_args(parser)


It looks like argb_add_default_args should be argp_add_default_args. Not sure why that one is argB.

zeroSteiner · 2018-10-16T13:24:02Z

KingPhisher

+			logger.warning("the following error occurred during pipenv environment setup: {}".format(process_output))
+			return 0
+	logger.debug('pipenv Pipfile: {}'.format(os.environ['PIPENV_PIPFILE']))
+	passing_argv = [' ', 'run', os.path.basename(__file__)] + sys_argv


Add a comment here that the ' ' is necessary.

zeroSteiner · 2018-10-16T13:25:21Z

king_phisher/client/__main__.py

@@ -0,0 +1,105 @@
+# -*- coding: utf-8 -*-
+#
+#  client/__main__.py


Should actually be king_phisher/client/__main__.py.

zeroSteiner · 2018-10-16T13:26:03Z

king_phisher/server/__main__.py

@@ -0,0 +1,243 @@
+# -*- coding: utf-8 -*-
+#
+#  server/__main__.py


Should actually be king_phisher/server/__main__.py.

zeroSteiner · 2018-10-16T13:27:35Z

king_phisher/startup.py

+
+def argp_add_wrapper(parser):
+	kpw_group = parser.add_argument_group('King Phisher pipenv wrapper')
+	kpw_group.add_argument('--env-update', dest='pipenv_update', default=False, action='store_true', help='update pipenv requirments and exit')


The word "requirments" is misspelled, it should be "requirements".

zeroSteiner · 2018-10-16T20:56:22Z

Landed with some minor changes in 2605856.

wolfthefallen added the enhancement Proposed improvements label Oct 8, 2018

wolfthefallen requested a review from zeroSteiner October 8, 2018 21:44

zeroSteiner self-assigned this Oct 9, 2018

zeroSteiner requested changes Oct 9, 2018

View reviewed changes

zeroSteiner added install Relating to the installation process in progress pinned Do not mark as stale labels Oct 9, 2018

zeroSteiner reviewed Oct 9, 2018

View reviewed changes

pipenv setup

1b41272

Pipenv install updates Set entries as the wrapper Updated basemaps to v1.2.0 pipenv pip18 fix add clean utilities Install pipenv from github Added gobject-interspection for package installs Pipenv enviroment

wolfthefallen force-pushed the venv branch from 5c20925 to 1b41272 Compare October 15, 2018 16:09

zeroSteiner requested changes Oct 16, 2018

View reviewed changes

pipenv updates

18591d9

zeroSteiner merged commit 18591d9 into rsmusllp:dev Oct 16, 2018

zeroSteiner added a commit that referenced this pull request Oct 16, 2018

Cleanups and a bug fix for PR #325

2605856

zeroSteiner added a commit that referenced this pull request Oct 16, 2018

Land #325, use Pipenv for managing the environment

bd7c668

zeroSteiner removed the in progress label Oct 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pipenv enviroment #325

pipenv enviroment #325

wolfthefallen commented Oct 8, 2018 •

edited by zeroSteiner

Loading

zeroSteiner left a comment

zeroSteiner Oct 9, 2018

zeroSteiner Oct 9, 2018

zeroSteiner Oct 9, 2018

zeroSteiner Oct 9, 2018

zeroSteiner Oct 9, 2018

zeroSteiner Oct 9, 2018

zeroSteiner Oct 9, 2018

zeroSteiner Oct 9, 2018

zeroSteiner Oct 9, 2018

zeroSteiner Oct 9, 2018

zeroSteiner Oct 9, 2018

zeroSteiner left a comment

zeroSteiner Oct 16, 2018

zeroSteiner Oct 16, 2018

zeroSteiner Oct 16, 2018

zeroSteiner Oct 16, 2018

zeroSteiner Oct 16, 2018

zeroSteiner Oct 16, 2018

zeroSteiner commented Oct 16, 2018

pipenv enviroment #325

pipenv enviroment #325

Conversation

wolfthefallen commented Oct 8, 2018 • edited by zeroSteiner Loading

zeroSteiner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeroSteiner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeroSteiner commented Oct 16, 2018

wolfthefallen commented Oct 8, 2018 •

edited by zeroSteiner

Loading