Skip to content

Commit

Permalink
Merge pull request #612 from tableau/jichikawa-dev-auth-warning
Browse files Browse the repository at this point in the history
Security: Require confirmation to start TabPy without authentication
  • Loading branch information
jakeichikawasalesforce authored Jun 21, 2023
2 parents 5716a67 + 59e3dbd commit 96aa262
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 12 deletions.
4 changes: 3 additions & 1 deletion .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ jobs:

strategy:
matrix:
python-version: [3.7, 3.8, 3.9]
# TODO: Add 3.7 to python-versions after GitHub action regression is resolved.
# https://github.com/actions/setup-python/issues/682
python-version: [3.8, 3.9]
os: [ubuntu-latest, windows-latest, macos-latest]

steps:
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ jobs:

strategy:
matrix:
python-version: [3.7, 3.8, 3.9]
# TODO: Add 3.7 to python-versions after GitHub action regression is resolved.
# https://github.com/actions/setup-python/issues/682
python-version: [3.8, 3.9]
os: [ubuntu-latest, windows-latest, macos-latest]

steps:
Expand Down
7 changes: 7 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Changelog

## v2.9.0

### Improvements

- Require confirmation to continue when starting TabPy without authentication,
with a warning that this is an insecure state and not recommended.

## v2.8.0

### Improvements
Expand Down
2 changes: 1 addition & 1 deletion tabpy/VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.8.0
2.9.0
13 changes: 9 additions & 4 deletions tabpy/tabpy.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
Usage:
tabpy [-h] | [--help]
tabpy [--config <CONFIG>]
tabpy [--config <CONFIG>] [--disable-auth-warning]
Options:
-h --help Show this screen.
--config <CONFIG> Path to a config file.
-h --help Show this screen.
--config <CONFIG> Path to a config file.
--disable-auth-warning Disable authentication warning.
"""

import docopt
Expand Down Expand Up @@ -38,9 +39,13 @@ def main():
args = docopt.docopt(__doc__)
config = args["--config"] or None

disable_auth_warning = False
if args["--disable-auth-warning"]:
disable_auth_warning = True

from tabpy.tabpy_server.app.app import TabPyApp

app = TabPyApp(config)
app = TabPyApp(config, disable_auth_warning)
app.run()


Expand Down
32 changes: 28 additions & 4 deletions tabpy/tabpy_server/app/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ class TabPyApp:
arrow_server = None
max_request_size = None

def __init__(self, config_file):
def __init__(self, config_file, disable_auth_warning=True):
self.disable_auth_warning = disable_auth_warning
if config_file is None:
config_file = os.path.join(
os.path.dirname(__file__), os.path.pardir, "common", "default.conf"
Expand Down Expand Up @@ -394,9 +395,7 @@ def _parse_config(self, config_file):
logger.critical(msg)
raise RuntimeError(msg)
else:
logger.info(
"Password file is not specified: " "Authentication is not enabled"
)
self._handle_configuration_without_authentication()

features = self._get_features()
self.settings[SettingsParameters.ApiVersions] = {"v1": {"features": features}}
Expand Down Expand Up @@ -471,6 +470,31 @@ def _parse_pwd_file(self):

return succeeded

def _handle_configuration_without_authentication(self):
std_no_auth_msg = "Password file is not specified: Authentication is not enabled"

if self.disable_auth_warning == True:
logger.info(std_no_auth_msg)
return

confirm_no_auth_msg = "\nWARNING: This TabPy server is not currently configured for username/password authentication. "

if self.settings[SettingsParameters.EvaluateEnabled]:
confirm_no_auth_msg += ("This means that, because the TABPY_EVALUATE_ENABLE feature is enabled, there is "
"the potential that unauthenticated individuals may be able to remotely execute code on this machine. ")

confirm_no_auth_msg += ("We strongly advise against proceeding without authentication as it poses a significant security risk.\n\n"
"Do you wish to proceed without authentication? (y/N): ")

confirm_no_auth_input = input(confirm_no_auth_msg)

if confirm_no_auth_input == 'y':
logger.info(std_no_auth_msg)
else:
print("\nAborting start up. To enable authentication for your TabPy server, see "
"https://github.com/tableau/TabPy/blob/master/docs/server-config.md#authentication.")
exit()

def _get_features(self):
features = {}

Expand Down
2 changes: 1 addition & 1 deletion tests/integration/integ_test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ def setUp(self):
# Platform specific - for integration tests we want to engage
# startup script
with open(self.tmp_dir + "/output.txt", "w") as outfile:
cmd = ["tabpy", "--config=" + self.config_file_name]
cmd = ["tabpy", "--config=" + self.config_file_name, "--disable-auth-warning"]
preexec_fn = None
if platform.system() == "Windows":
self.py = "python"
Expand Down
20 changes: 20 additions & 0 deletions tests/unit/server_tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,26 @@ def test_no_state_ini_file_or_state_dir(
TabPyApp(None)
self.assertEqual(len(mock_os.makedirs.mock_calls), 1)

@patch('builtins.input', return_value='y')
@patch("tabpy.tabpy_server.app.app.os")
@patch("tabpy.tabpy_server.app.app.os.path.exists", return_value=False)
@patch("tabpy.tabpy_server.app.app.PythonServiceHandler")
@patch("tabpy.tabpy_server.app.app._get_state_from_file")
@patch("tabpy.tabpy_server.app.app.TabPyState")
def test_handle_configuration_without_authentication(
self,
mock_tabpy_state,
mock_get_state_from_file,
mock_psws,
mock_os_path_exists,
mock_os,
mock_input,
):
TabPyApp(None)
mock_input.assert_not_called()

TabPyApp(None, False)
mock_input.assert_called()

class TestPartialConfigFile(unittest.TestCase):
def setUp(self):
Expand Down

0 comments on commit 96aa262

Please sign in to comment.