sin/samba-mirror
1
0
Fork 0
mirror of https://github.com/samba-team/samba.git synced 2025-01-11 05:18:09 +03:00
Code

netcmd: don't turn exception into CommandError in run_validators

Browse Source 
It's the wrong place to do it.

Instead, let it raise the original exception, capture it in _run, and
call existing show_command_error method.

Signed-off-by: Rob van der Linde <rob@catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
This commit is contained in:
Rob van der Linde 2023-10-05 14:03:14 +13:00 committed by Andrew Bartlett
parent 99c93c1e89
commit bdad257a31
2 changed files with 70 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
python/samba/netcmd/__init__.py
View File

@ -31,7 +31,6 @@ from samba.logger import get_samba_logger
from samba.samdb import SamDB
from .encoders import JSONEncoder
from .validators import ValidationError
class Option(SambaOption):
@ -39,18 +38,10 @@ class Option(SambaOption):
SUPPRESS_HELP = optparse.SUPPRESS_HELP
def run_validators(self, opt, value):
"""Runs the list of validators on the current option.
If the validator raises ValidationError, turn that into CommandError
which gives nicer output.
"""
"""Runs the list of validators on the current option."""
validators = getattr(self, "validators") or []
for validator in validators:
try:
validator(opt, value)
except ValidationError as e:
raise CommandError(e)
validator(opt, value)
def convert_value(self, opt, value):
"""Override convert_value to run validators just after.
@ -242,7 +233,14 @@ class Command(object):
def _run(self, *argv):
parser, optiongroups = self._create_parser(self.command_name)
opts, args = parser.parse_args(list(argv))
# Handle possible validation errors raised by parser
try:
opts, args = parser.parse_args(list(argv))
except Exception as e:
self.show_command_error(e)
return -1
# Filter out options from option groups
kwargs = dict(opts.__dict__)
for option_group in parser.option_groups:

133
python/samba/tests/samba_tool/domain_auth_policy.py
View File

@ -26,7 +26,6 @@ from unittest.mock import patch
from samba.dcerpc import security
from samba.ndr import ndr_unpack
from samba.netcmd import CommandError
from samba.netcmd.domain.models.exceptions import ModelError
from samba.samdb import SamDB
from samba.sd_utils import SDUtils
@ -141,22 +140,20 @@ class AuthPolicyCmdTestCase(BaseAuthCmdTest):
self.assertEqual(str(policy["msDS-UserTGTLifetime"]), "60")
# check lower bounds (45)
with self.assertRaises(CommandError) as e:
self.runcmd("domain", "auth", "policy", "create",
"--name", "userTGTLifetimeLower",
"--user-tgt-lifetime", "44")
result, out, err = self.runcmd("domain", "auth", "policy", "create",
"--name", "userTGTLifetimeLower",
"--user-tgt-lifetime", "44")
self.assertEqual(result, -1)
self.assertIn("--user-tgt-lifetime must be between 45 and 2147483647",
str(e.exception))
err)
# check upper bounds (2147483647)
with self.assertRaises(CommandError) as e:
self.runcmd("domain", "auth", "policy", "create",
"--name", "userTGTLifetimeUpper",
"--user-tgt-lifetime", "2147483648")
result, out, err = self.runcmd("domain", "auth", "policy", "create",
"--name", "userTGTLifetimeUpper",
"--user-tgt-lifetime", "2147483648")
self.assertEqual(result, -1)
self.assertIn("--user-tgt-lifetime must be between 45 and 2147483647",
str(e.exception))
err)
def test_authentication_policy_create_service_tgt_lifetime(self):
"""Test create a new authentication policy with --service-tgt-lifetime.
@ -177,22 +174,20 @@ class AuthPolicyCmdTestCase(BaseAuthCmdTest):
self.assertEqual(str(policy["msDS-ServiceTGTLifetime"]), "60")
# check lower bounds (45)
with self.assertRaises(CommandError) as e:
self.runcmd("domain", "auth", "policy", "create",
"--name", "serviceTGTLifetimeLower",
"--service-tgt-lifetime", "44")
result, out, err = self.runcmd("domain", "auth", "policy", "create",
"--name", "serviceTGTLifetimeLower",
"--service-tgt-lifetime", "44")
self.assertEqual(result, -1)
self.assertIn("--service-tgt-lifetime must be between 45 and 2147483647",
str(e.exception))
err)
# check upper bounds (2147483647)
with self.assertRaises(CommandError) as e:
self.runcmd("domain", "auth", "policy", "create",
"--name", "serviceTGTLifetimeUpper",
"--service-tgt-lifetime", "2147483648")
result, out, err = self.runcmd("domain", "auth", "policy", "create",
"--name", "serviceTGTLifetimeUpper",
"--service-tgt-lifetime", "2147483648")
self.assertEqual(result, -1)
self.assertIn("--service-tgt-lifetime must be between 45 and 2147483647",
str(e.exception))
err)
def test_authentication_policy_create_computer_tgt_lifetime(self):
"""Test create a new authentication policy with --computer-tgt-lifetime.
@ -213,22 +208,20 @@ class AuthPolicyCmdTestCase(BaseAuthCmdTest):
self.assertEqual(str(policy["msDS-ComputerTGTLifetime"]), "60")
# check lower bounds (45)
with self.assertRaises(CommandError) as e:
self.runcmd("domain", "auth", "policy", "create",
"--name", "computerTGTLifetimeLower",
"--computer-tgt-lifetime", "44")
result, out, err = self.runcmd("domain", "auth", "policy", "create",
"--name", "computerTGTLifetimeLower",
"--computer-tgt-lifetime", "44")
self.assertEqual(result, -1)
self.assertIn("--computer-tgt-lifetime must be between 45 and 2147483647",
str(e.exception))
err)
# check upper bounds (2147483647)
with self.assertRaises(CommandError) as e:
self.runcmd("domain", "auth", "policy", "create",
"--name", "computerTGTLifetimeUpper",
"--computer-tgt-lifetime", "2147483648")
result, out, err = self.runcmd("domain", "auth", "policy", "create",
"--name", "computerTGTLifetimeUpper",
"--computer-tgt-lifetime", "2147483648")
self.assertEqual(result, -1)
self.assertIn("--computer-tgt-lifetime must be between 45 and 2147483647",
str(e.exception))
err)
def test_authentication_policy_create_valid_sddl(self):
"""Test creating a new authentication policy with valid SDDL in a field."""
@ -387,22 +380,20 @@ class AuthPolicyCmdTestCase(BaseAuthCmdTest):
self.assertEqual(str(policy["msDS-UserTGTLifetime"]), "120")
# check lower bounds (45)
with self.assertRaises(CommandError) as e:
self.runcmd("domain", "auth", "policy", "modify",
"--name", name,
"--user-tgt-lifetime", "44")
result, out, err = self.runcmd("domain", "auth", "policy", "modify",
"--name", name,
"--user-tgt-lifetime", "44")
self.assertEqual(result, -1)
self.assertIn("--user-tgt-lifetime must be between 45 and 2147483647",
str(e.exception))
err)
# check upper bounds (2147483647)
with self.assertRaises(CommandError) as e:
self.runcmd("domain", "auth", "policy", "modify",
"--name", name,
"--user-tgt-lifetime", "2147483648")
result, out, err = self.runcmd("domain", "auth", "policy", "modify",
"--name", name,
"--user-tgt-lifetime", "2147483648")
self.assertEqual(result, -1)
self.assertIn("-user-tgt-lifetime must be between 45 and 2147483647",
str(e.exception))
err)
def test_authentication_policy_modify_service_tgt_lifetime(self):
"""Test modifying an authentication policy --service-tgt-lifetime.
@ -425,22 +416,20 @@ class AuthPolicyCmdTestCase(BaseAuthCmdTest):
self.assertEqual(str(policy["msDS-ServiceTGTLifetime"]), "120")
# check lower bounds (45)
with self.assertRaises(CommandError) as e:
self.runcmd("domain", "auth", "policy", "modify",
"--name", name,
"--service-tgt-lifetime", "44")
result, out, err = self.runcmd("domain", "auth", "policy", "modify",
"--name", name,
"--service-tgt-lifetime", "44")
self.assertEqual(result, -1)
self.assertIn("--service-tgt-lifetime must be between 45 and 2147483647",
str(e.exception))
err)
# check upper bounds (2147483647)
with self.assertRaises(CommandError) as e:
self.runcmd("domain", "auth", "policy", "modify",
"--name", name,
"--service-tgt-lifetime", "2147483648")
result, out, err = self.runcmd("domain", "auth", "policy", "modify",
"--name", name,
"--service-tgt-lifetime", "2147483648")
self.assertEqual(result, -1)
self.assertIn("--service-tgt-lifetime must be between 45 and 2147483647",
str(e.exception))
err)
def test_authentication_policy_modify_computer_tgt_lifetime(self):
"""Test modifying an authentication policy --computer-tgt-lifetime.
@ -463,22 +452,20 @@ class AuthPolicyCmdTestCase(BaseAuthCmdTest):
self.assertEqual(str(policy["msDS-ComputerTGTLifetime"]), "120")
# check lower bounds (45)
with self.assertRaises(CommandError) as e:
self.runcmd("domain", "auth", "policy", "modify",
"--name", name,
"--computer-tgt-lifetime", "44")
result, out, err = self.runcmd("domain", "auth", "policy", "modify",
"--name", name,
"--computer-tgt-lifetime", "44")
self.assertEqual(result, -1)
self.assertIn("--computer-tgt-lifetime must be between 45 and 2147483647",
str(e.exception))
err)
# check upper bounds (2147483647)
with self.assertRaises(CommandError) as e:
self.runcmd("domain", "auth", "policy", "modify",
"--name", name,
"--computer-tgt-lifetime", "2147483648")
result, out, err = self.runcmd("domain", "auth", "policy", "modify",
"--name", name,
"--computer-tgt-lifetime", "2147483648")
self.assertEqual(result, -1)
self.assertIn("--computer-tgt-lifetime must be between 45 and 2147483647",
str(e.exception))
err)
def test_authentication_policy_modify_name_missing(self):
"""Test modify authentication but the --name argument is missing."""