Skip to content

Commit 505fa14

Browse files
author
Dean Troyer
committed
Fix auth-required for help command
When we got picky with the auth arguments we broke using help without any auth config supplied. This rearranges things a bit to do the argument checking when the deferred auth request to Identity occurs so commands that do not need auth have a chance to live short but useful lives. Closes-Bug: #1399588 Change-Id: I8ceac491cf65e25eddb62ab2713f471fe686756d
1 parent 9400eff commit 505fa14

5 files changed

Lines changed: 90 additions & 60 deletions

File tree

‎examples/osc-lib.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ def run(opts):
5959
# Collect the auth and config options together and give them to
6060
# ClientManager and it will wrangle all of the goons into place.
6161
client_manager = clientmanager.ClientManager(
62-
auth_options=opts,
62+
cli_options=opts,
6363
verify=verify,
6464
api_version=api_version,
6565
)

‎openstackclient/api/auth.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,9 @@ def select_auth_plugin(options):
8585
# let keystoneclient figure it out itself
8686
auth_plugin_name = 'token'
8787
else:
88-
raise exc.CommandError(
89-
"Authentication type must be selected with --os-auth-type"
90-
)
88+
# The ultimate default is similar to the original behaviour,
89+
# but this time with version discovery
90+
auth_plugin_name = 'osc_password'
9191
LOG.debug("Auth plugin %s selected" % auth_plugin_name)
9292
return auth_plugin_name
9393

‎openstackclient/common/clientmanager.py‎

Lines changed: 42 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,14 @@ def __getattr__(self, name):
5656

5757
def __init__(
5858
self,
59-
auth_options,
59+
cli_options,
6060
api_version=None,
6161
verify=True,
6262
pw_func=None,
6363
):
6464
"""Set up a ClientManager
6565
66-
:param auth_options:
66+
:param cli_options:
6767
Options collected from the command-line, environment, or wherever
6868
:param api_version:
6969
Dict of API versions: key is API name, value is the version
@@ -77,31 +77,57 @@ def __init__(
7777
returns a string containing the password
7878
"""
7979

80+
self._cli_options = cli_options
81+
self._api_version = api_version
82+
self._pw_callback = pw_func
83+
self._url = self._cli_options.os_url
84+
self._region_name = self._cli_options.os_region_name
85+
86+
self.timing = self._cli_options.timing
87+
88+
self._auth_ref = None
89+
self.session = None
90+
91+
# verify is the Requests-compatible form
92+
self._verify = verify
93+
# also store in the form used by the legacy client libs
94+
self._cacert = None
95+
if isinstance(verify, bool):
96+
self._insecure = not verify
97+
else:
98+
self._cacert = verify
99+
self._insecure = False
100+
101+
# Get logging from root logger
102+
root_logger = logging.getLogger('')
103+
LOG.setLevel(root_logger.getEffectiveLevel())
104+
105+
def setup_auth(self):
106+
"""Set up authentication
107+
108+
This is deferred until authentication is actually attempted because
109+
it gets in the way of things that do not require auth.
110+
"""
111+
80112
# If no auth type is named by the user, select one based on
81113
# the supplied options
82-
self.auth_plugin_name = auth.select_auth_plugin(auth_options)
114+
self.auth_plugin_name = auth.select_auth_plugin(self._cli_options)
83115

84116
# Basic option checking to avoid unhelpful error messages
85-
auth.check_valid_auth_options(auth_options, self.auth_plugin_name)
117+
auth.check_valid_auth_options(self._cli_options, self.auth_plugin_name)
86118

87119
# Horrible hack alert...must handle prompt for null password if
88120
# password auth is requested.
89121
if (self.auth_plugin_name.endswith('password') and
90-
not auth_options.os_password):
91-
auth_options.os_password = pw_func()
122+
not self._cli_options.os_password):
123+
self._cli_options.os_password = self.pw_callback()
92124

93125
(auth_plugin, self._auth_params) = auth.build_auth_params(
94126
self.auth_plugin_name,
95-
auth_options,
127+
self._cli_options,
96128
)
97129

98-
self._url = auth_options.os_url
99-
self._region_name = auth_options.os_region_name
100-
self._api_version = api_version
101-
self._auth_ref = None
102-
self.timing = auth_options.timing
103-
104-
default_domain = auth_options.os_default_domain
130+
default_domain = self._cli_options.os_default_domain
105131
# NOTE(stevemar): If PROJECT_DOMAIN_ID or PROJECT_DOMAIN_NAME is
106132
# present, then do not change the behaviour. Otherwise, set the
107133
# PROJECT_DOMAIN_ID to 'OS_DEFAULT_DOMAIN' for better usability.
@@ -125,28 +151,14 @@ def __init__(
125151
elif 'tenant_name' in self._auth_params:
126152
self._project_name = self._auth_params['tenant_name']
127153

128-
# verify is the Requests-compatible form
129-
self._verify = verify
130-
# also store in the form used by the legacy client libs
131-
self._cacert = None
132-
if isinstance(verify, bool):
133-
self._insecure = not verify
134-
else:
135-
self._cacert = verify
136-
self._insecure = False
137-
138-
# Get logging from root logger
139-
root_logger = logging.getLogger('')
140-
LOG.setLevel(root_logger.getEffectiveLevel())
141-
142154
LOG.info('Using auth plugin: %s' % self.auth_plugin_name)
143155
self.auth = auth_plugin.load_from_options(**self._auth_params)
144156
# needed by SAML authentication
145157
request_session = requests.session()
146158
self.session = session.Session(
147159
auth=self.auth,
148160
session=request_session,
149-
verify=verify,
161+
verify=self._verify,
150162
)
151163

152164
return
@@ -155,6 +167,7 @@ def __init__(
155167
def auth_ref(self):
156168
"""Dereference will trigger an auth if it hasn't already"""
157169
if not self._auth_ref:
170+
self.setup_auth()
158171
LOG.debug("Get auth_ref")
159172
self._auth_ref = self.auth.get_auth_ref(self.session)
160173
return self._auth_ref

‎openstackclient/shell.py‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
from cliff import app
2424
from cliff import command
25-
from cliff import complete
2625
from cliff import help
2726

2827
import openstackclient
@@ -70,10 +69,9 @@ class OpenStackShell(app.App):
7069
def __init__(self):
7170
# Patch command.Command to add a default auth_required = True
7271
command.Command.auth_required = True
73-
command.Command.best_effort = False
74-
# But not help
72+
73+
# Some commands do not need authentication
7574
help.HelpCommand.auth_required = False
76-
complete.CompleteCommand.best_effort = True
7775

7876
super(OpenStackShell, self).__init__(
7977
description=__doc__.strip(),
@@ -294,7 +292,7 @@ def initialize_app(self, argv):
294292
self.verify = not self.options.insecure
295293

296294
self.client_manager = clientmanager.ClientManager(
297-
auth_options=self.options,
295+
cli_options=self.options,
298296
verify=self.verify,
299297
api_version=self.api_version,
300298
pw_func=prompt_for_password,
@@ -308,7 +306,7 @@ def prepare_to_run_command(self, cmd):
308306
cmd.__class__.__module__,
309307
cmd.__class__.__name__,
310308
)
311-
if cmd.auth_required and cmd.best_effort:
309+
if cmd.auth_required:
312310
try:
313311
# Trigger the Identity client to initialize
314312
self.client_manager.auth_ref

‎openstackclient/tests/common/test_clientmanager.py‎

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,16 @@ def setUp(self):
8080
def test_client_manager_token_endpoint(self):
8181

8282
client_manager = clientmanager.ClientManager(
83-
auth_options=FakeOptions(os_token=fakes.AUTH_TOKEN,
84-
os_url=fakes.AUTH_URL,
85-
os_auth_type='token_endpoint'),
83+
cli_options=FakeOptions(
84+
os_token=fakes.AUTH_TOKEN,
85+
os_url=fakes.AUTH_URL,
86+
os_auth_type='token_endpoint',
87+
),
8688
api_version=API_VERSION,
8789
verify=True
8890
)
91+
client_manager.setup_auth()
92+
8993
self.assertEqual(
9094
fakes.AUTH_URL,
9195
client_manager._url,
@@ -104,12 +108,15 @@ def test_client_manager_token_endpoint(self):
104108
def test_client_manager_token(self):
105109

106110
client_manager = clientmanager.ClientManager(
107-
auth_options=FakeOptions(os_token=fakes.AUTH_TOKEN,
108-
os_auth_url=fakes.AUTH_URL,
109-
os_auth_type='v2token'),
111+
cli_options=FakeOptions(
112+
os_token=fakes.AUTH_TOKEN,
113+
os_auth_url=fakes.AUTH_URL,
114+
os_auth_type='v2token',
115+
),
110116
api_version=API_VERSION,
111117
verify=True
112118
)
119+
client_manager.setup_auth()
113120

114121
self.assertEqual(
115122
fakes.AUTH_URL,
@@ -125,13 +132,16 @@ def test_client_manager_token(self):
125132
def test_client_manager_password(self):
126133

127134
client_manager = clientmanager.ClientManager(
128-
auth_options=FakeOptions(os_auth_url=fakes.AUTH_URL,
129-
os_username=fakes.USERNAME,
130-
os_password=fakes.PASSWORD,
131-
os_project_name=fakes.PROJECT_NAME),
135+
cli_options=FakeOptions(
136+
os_auth_url=fakes.AUTH_URL,
137+
os_username=fakes.USERNAME,
138+
os_password=fakes.PASSWORD,
139+
os_project_name=fakes.PROJECT_NAME,
140+
),
132141
api_version=API_VERSION,
133142
verify=False,
134143
)
144+
client_manager.setup_auth()
135145

136146
self.assertEqual(
137147
fakes.AUTH_URL,
@@ -182,14 +192,17 @@ def stub_auth(self, json=None, url=None, verb=None, **kwargs):
182192
def test_client_manager_password_verify_ca(self):
183193

184194
client_manager = clientmanager.ClientManager(
185-
auth_options=FakeOptions(os_auth_url=fakes.AUTH_URL,
186-
os_username=fakes.USERNAME,
187-
os_password=fakes.PASSWORD,
188-
os_project_name=fakes.PROJECT_NAME,
189-
os_auth_type='v2password'),
195+
cli_options=FakeOptions(
196+
os_auth_url=fakes.AUTH_URL,
197+
os_username=fakes.USERNAME,
198+
os_password=fakes.PASSWORD,
199+
os_project_name=fakes.PROJECT_NAME,
200+
os_auth_type='v2password',
201+
),
190202
api_version=API_VERSION,
191203
verify='cafile',
192204
)
205+
client_manager.setup_auth()
193206

194207
self.assertFalse(client_manager._insecure)
195208
self.assertTrue(client_manager._verify)
@@ -199,10 +212,12 @@ def _select_auth_plugin(self, auth_params, api_version, auth_plugin_name):
199212
auth_params['os_auth_type'] = auth_plugin_name
200213
auth_params['os_identity_api_version'] = api_version
201214
client_manager = clientmanager.ClientManager(
202-
auth_options=FakeOptions(**auth_params),
215+
cli_options=FakeOptions(**auth_params),
203216
api_version=API_VERSION,
204217
verify=True
205218
)
219+
client_manager.setup_auth()
220+
206221
self.assertEqual(
207222
auth_plugin_name,
208223
client_manager.auth_plugin_name,
@@ -228,8 +243,12 @@ def test_client_manager_select_auth_plugin(self):
228243
self._select_auth_plugin(params, 'XXX', 'password')
229244

230245
def test_client_manager_select_auth_plugin_failure(self):
231-
self.assertRaises(exc.CommandError,
232-
clientmanager.ClientManager,
233-
auth_options=FakeOptions(os_auth_plugin=''),
234-
api_version=API_VERSION,
235-
verify=True)
246+
client_manager = clientmanager.ClientManager(
247+
cli_options=FakeOptions(os_auth_plugin=''),
248+
api_version=API_VERSION,
249+
verify=True,
250+
)
251+
self.assertRaises(
252+
exc.CommandError,
253+
client_manager.setup_auth,
254+
)

0 commit comments

Comments
 (0)