Skip to content

Commit

Permalink
[minor] bandit flagging and some py2 cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
gcoxmoz committed Dec 26, 2024
1 parent 4ccd31f commit a08f848
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 42 deletions.
1 change: 0 additions & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ Features
- Simple. Sort of.
- Fail open (optional).
- OTP and Push (use push as password for push, passcode:123456 as password for OTP, where 123456 is your OTP).
- MozDef support.
- Optional-username hack, in case you use emails as certificate CN but only the first part of the email as login.
- Deferred call.

Expand Down
6 changes: 2 additions & 4 deletions duo_openvpn_mozilla/openvpn_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,9 @@ def load_variables_from_environment(self):
__unsafe_username = ''

# the environmental variable 'password' is what they typed in/sent.
__password = os.environ.get('password')
if __password is None:
__password = ''
__password = os.environ.get('password', '')

if __unsafe_username != '' and __password == '':
if __unsafe_username != '' and __password == '': # nosec hardcoded_password_string
# At this step, we have a username of some sort, but no password.
#
# Based on changes dating back to 2015, as an aid to
Expand Down
10 changes: 5 additions & 5 deletions test/integration/test_duo_openvpn.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def test_init(self):
def test_bogus_user(self):
""" A bogus user is denied """
os.environ['common_name'] = 'user-who-does-not-exist'
os.environ['password'] = 'push'
os.environ['password'] = 'push' # nosec hardcoded_password_string
library = DuoOpenVPN()
res = library.main_authentication()
self.assertFalse(res, 'invalid users must be denied')
Expand All @@ -81,7 +81,7 @@ def test_1fa_user_attempts_2fa(self):
if not self.one_fa_user: # pragma: no cover
return self.skipTest('No testing/one_fa_user defined')
os.environ['common_name'] = self.one_fa_user
os.environ['password'] = 'push'
os.environ['password'] = 'push' # nosec hardcoded_password_string
library = DuoOpenVPN()
res = library.main_authentication()
self.assertFalse(res, '1fa user attempting to 2fa must be denied')
Expand All @@ -91,7 +91,7 @@ def test_1fa_user_bad_pw(self):
if not self.one_fa_user: # pragma: no cover
return self.skipTest('No testing/one_fa_user defined')
os.environ['common_name'] = self.one_fa_user
os.environ['password'] = 'a-bad-password'
os.environ['password'] = 'a-bad-password' # nosec hardcoded_password_string
library = DuoOpenVPN()
res = library.main_authentication()
self.assertFalse(res, '1fa user with bad password must be denied')
Expand All @@ -115,7 +115,7 @@ def test_2fa_user_bad(self):
if not self.normal_user: # pragma: no cover
return self.skipTest('No testing/normal_user defined')
os.environ['common_name'] = self.normal_user
os.environ['password'] = 'push'
os.environ['password'] = 'push' # nosec hardcoded_password_string
library = DuoOpenVPN()
res = library.main_authentication()
self.assertFalse(res, '2fa user with a deny must be False')
Expand All @@ -127,7 +127,7 @@ def test_2fa_user_good(self):
if not self.normal_user: # pragma: no cover
return self.skipTest('No testing/normal_user defined')
os.environ['common_name'] = self.normal_user
os.environ['password'] = 'push'
os.environ['password'] = 'push' # nosec hardcoded_password_string
library = DuoOpenVPN()
res = library.main_authentication()
self.assertTrue(res, '2fa user with an allow must be True')
Expand Down
4 changes: 2 additions & 2 deletions test/unit/test_duo_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class TestDuoAPIAuthUnit(unittest.TestCase):
DuoAPIAuth class without going out to Duo.
"""

testing_conffile = '/tmp/TestDuoAPIAuthUnit.txt'
testing_conffile = '/tmp/TestDuoAPIAuthUnit.txt' # nosec hardcoded_tmp_directory

def setUp(self):
""" Preparing test rig """
Expand Down Expand Up @@ -288,7 +288,7 @@ def test_33_auth_auto(self):

def test_34_auth_passcode(self):
""" Test passcode on auth checks. """
os.environ['password'] = '123456'
os.environ['password'] = '123456' # nosec hardcoded_password_string
creds = OpenVPNCredentials()
creds.load_variables_from_environment()
self.library.user_config = creds
Expand Down
22 changes: 10 additions & 12 deletions test/unit/test_duo_openvpn.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,16 @@

import unittest
import os
import sys
import datetime
import json
import syslog
import configparser
from io import StringIO
import test.context # pylint: disable=unused-import
import mock
from duo_openvpn_mozilla.duo_auth import DuoAPIAuth
from duo_openvpn_mozilla.openvpn_credentials import OpenVPNCredentials
from duo_openvpn_mozilla import DuoOpenVPN, DuoTimeoutError
if sys.version_info.major >= 3:
from io import StringIO # pragma: no cover
else:
from io import BytesIO as StringIO # pragma: no cover


class TestDuoOpenVPNUnit(unittest.TestCase):
Expand All @@ -28,7 +24,7 @@ class TestDuoOpenVPNUnit(unittest.TestCase):
DuoOpenVPN class without going out to Duo.
"""

testing_conffile = '/tmp/TestDuoOpenVPNUnit.txt'
testing_conffile = '/tmp/TestDuoOpenVPNUnit.txt' # nosec hardcoded_tmp_directory

def setUp(self):
""" Preparing test rig """
Expand Down Expand Up @@ -66,8 +62,9 @@ def test_03_ingest_no_config_files(self):

def test_04_ingest_no_config_file(self):
""" With all missing config files, get an exception """
_not_a_real_file = '/tmp/no-such-file.txt' # nosec hardcoded_tmp_directory
with mock.patch.object(DuoOpenVPN, 'CONFIG_FILE_LOCATIONS',
new=['/tmp/no-such-file.txt']):
new=[_not_a_real_file]):
with self.assertRaises(IOError):
self.library._ingest_config_from_file()

Expand All @@ -80,12 +77,13 @@ def test_05_ingest_bad_config_file(self):

def test_06_ingest_config_from_file(self):
""" With an actual config file, get a populated ConfigParser """
test_reading_file = '/tmp/test-reader.txt'
_not_a_real_file = '/tmp/no-such-file.txt' # nosec hardcoded_tmp_directory
test_reading_file = '/tmp/test-reader.txt' # nosec hardcoded_tmp_directory
with open(test_reading_file, 'w', encoding='utf-8') as filepointer:
filepointer.write('[aa]\nbb = cc\n')
filepointer.close()
with mock.patch.object(DuoOpenVPN, 'CONFIG_FILE_LOCATIONS',
new=['/tmp/no-such-file.txt', test_reading_file]):
new=[_not_a_real_file, test_reading_file]):
result = self.library._ingest_config_from_file()
os.remove(test_reading_file)
self.assertIsInstance(result, configparser.ConfigParser,
Expand Down Expand Up @@ -248,7 +246,7 @@ def test_23_auth_1fa_garbage_pw(self):
""" 1fa user with a 2fa / bonkers 'password' """
os.environ['untrusted_ip'] = 'testing-ip-Unknown-is-OK'
os.environ['common_name'] = 'bob'
os.environ['password'] = 'push'
os.environ['password'] = 'push' # nosec hardcoded_password_string
with mock.patch.object(DuoOpenVPN, 'log') as mock_log:
with mock.patch('iamvpnlibrary.IAMVPNLibrary') as mock_iam, \
mock.patch.object(mock_iam.return_value, 'user_allowed_to_vpn',
Expand All @@ -264,7 +262,7 @@ def test_24_auth_1fa_bad_pw(self):
""" 1fa user with a bad password """
os.environ['untrusted_ip'] = 'testing-ip-Unknown-is-OK'
os.environ['common_name'] = 'bob'
os.environ['password'] = 'hunter2'
os.environ['password'] = 'hunter2' # nosec hardcoded_password_string
with mock.patch.object(DuoOpenVPN, 'log') as mock_log:
with mock.patch('iamvpnlibrary.IAMVPNLibrary') as mock_iam, \
mock.patch.object(mock_iam.return_value, 'user_allowed_to_vpn',
Expand All @@ -282,7 +280,7 @@ def test_25_auth_1fa_good_pw(self):
""" 1fa user with a good password """
os.environ['untrusted_ip'] = 'testing-ip-Unknown-is-OK'
os.environ['common_name'] = 'bob'
os.environ['password'] = 'hunter2'
os.environ['password'] = 'hunter2' # nosec hardcoded_password_string
with mock.patch.object(DuoOpenVPN, 'log') as mock_log:
with mock.patch('iamvpnlibrary.IAMVPNLibrary') as mock_iam, \
mock.patch.object(mock_iam.return_value, 'user_allowed_to_vpn',
Expand Down
18 changes: 9 additions & 9 deletions test/unit/test_openvpn_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ def test_loadvar_00(self):
""" user '' pass '' """
os.environ['common_name'] = self.realish_user
os.environ['username'] = ''
os.environ['password'] = ''
os.environ['password'] = '' # nosec hardcoded_password_string
self.library.load_variables_from_environment()
self.assertTrue(self.library.is_valid())
self.assertEqual(self.library.username, self.realish_user)
Expand Down Expand Up @@ -243,7 +243,7 @@ def test_loadvar_05(self):

def test_loadvar_06(self):
""" user '' pass a_pa:ss:wo:rd """
_pass = 'some:password'
_pass = 'some:password' # nosec hardcoded_password_string
os.environ['common_name'] = self.realish_user
os.environ['username'] = ''
os.environ['password'] = _pass
Expand All @@ -256,7 +256,7 @@ def test_loadvar_06(self):

def test_loadvar_07(self):
""" user '' pass a_password """
_pass = 'some_password'
_pass = 'some_password' # nosec hardcoded_password_string
os.environ['common_name'] = self.realish_user
os.environ['username'] = ''
os.environ['password'] = _pass
Expand All @@ -277,7 +277,7 @@ def test_loadvar_10(self):
for wordu in list(self.library.DUO_RESERVED_WORDS):
os.environ['common_name'] = self.realish_user
os.environ['username'] = wordu
os.environ['password'] = ''
os.environ['password'] = '' # nosec hardcoded_password_string
self.library.load_variables_from_environment()
self.assertTrue(self.library.is_valid())
self.assertEqual(self.library.username, self.realish_user)
Expand Down Expand Up @@ -359,7 +359,7 @@ def test_loadvar_15(self):

def test_loadvar_16(self):
""" user DUO_RESERVED_WORDS pass a_pa:ss:wo:rd """
_pass = 'some:password'
_pass = 'some:password' # nosec hardcoded_password_string
for wordu in list(self.library.DUO_RESERVED_WORDS):
os.environ['common_name'] = self.realish_user
os.environ['username'] = wordu
Expand All @@ -373,7 +373,7 @@ def test_loadvar_16(self):

def test_loadvar_17(self):
""" user DUO_RESERVED_WORDS pass a_password """
_pass = 'some_password'
_pass = 'some_password' # nosec hardcoded_password_string
for wordu in list(self.library.DUO_RESERVED_WORDS):
os.environ['common_name'] = self.realish_user
os.environ['username'] = wordu
Expand Down Expand Up @@ -403,7 +403,7 @@ def test_loadvar_20(self):
# or have a non-factor result. So:
os.environ['common_name'] = self.realish_user
os.environ['username'] = 'someuser'
os.environ['password'] = ''
os.environ['password'] = '' # nosec hardcoded_password_string
try:
self.library.load_variables_from_environment()
except ValueError: # pragma: no cover
Expand Down Expand Up @@ -487,7 +487,7 @@ def test_loadvar_25(self):

def test_loadvar_26(self):
""" user someuser pass a_pa:ss:wo:rd """
_pass = 'some:password'
_pass = 'some:password' # nosec hardcoded_password_string
os.environ['common_name'] = self.realish_user
os.environ['username'] = 'someuser'
os.environ['password'] = _pass
Expand All @@ -500,7 +500,7 @@ def test_loadvar_26(self):

def test_loadvar_27(self):
""" user someuser pass a_password """
_pass = 'some_password'
_pass = 'some_password' # nosec hardcoded_password_string
os.environ['common_name'] = self.realish_user
os.environ['username'] = 'someuser'
os.environ['password'] = _pass
Expand Down
14 changes: 5 additions & 9 deletions test/unit/test_openvpn_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,12 @@

import unittest
import os
import sys
import time
import test.context # pylint: disable=unused-import
from io import StringIO
import mock
import duo_openvpn
from duo_openvpn_mozilla import DuoOpenVPN # pylint: disable=unused-import
if sys.version_info.major >= 3:
from io import StringIO # pragma: no cover
else:
from io import BytesIO as StringIO # pragma: no cover


class TestOpenVPNScript(unittest.TestCase):
Expand All @@ -32,7 +28,7 @@ def test_01_no_control_file(self):

def test_05_write_failed(self):
""" When we can't write out auth_control_file, we must die """
os.environ['auth_control_file'] = '/tmp/foo'
os.environ['auth_control_file'] = '/tmp/foo' # nosec hardcoded_tmp_directory
with mock.patch('duo_openvpn.DuoOpenVPN'), \
self.assertRaises(SystemExit) as exiting, \
mock.patch('duo_openvpn.open', create=True, side_effect=IOError):
Expand All @@ -41,7 +37,7 @@ def test_05_write_failed(self):

def test_10_access_denied(self):
""" When auth is refused, don't let someone in. """
os.environ['auth_control_file'] = '/tmp/foo'
os.environ['auth_control_file'] = '/tmp/foo' # nosec hardcoded_tmp_directory
with mock.patch('duo_openvpn.DuoOpenVPN') as mock_duo, \
self.assertRaises(SystemExit) as exiting, \
mock.patch('duo_openvpn.open', create=True,
Expand All @@ -54,7 +50,7 @@ def test_10_access_denied(self):

def test_11_access_granted(self):
""" When auth is allowed, let someone in. """
os.environ['auth_control_file'] = '/tmp/foo'
os.environ['auth_control_file'] = '/tmp/foo' # nosec hardcoded_tmp_directory
with mock.patch('duo_openvpn.DuoOpenVPN') as mock_duo, \
self.assertRaises(SystemExit) as exiting, \
mock.patch('duo_openvpn.open', create=True,
Expand All @@ -67,7 +63,7 @@ def test_11_access_granted(self):

def test_12_access_timeout(self):
""" When Duo is down, handle it well. """
os.environ['auth_control_file'] = '/tmp/foo'
os.environ['auth_control_file'] = '/tmp/foo' # nosec hardcoded_tmp_directory
def too_slow_authentication():
'''
This function is waiting 5 seconds and the responding true.
Expand Down

0 comments on commit a08f848

Please sign in to comment.