Skip to content

Commit 496e78e

Browse files
at88mphbsipocz
authored andcommitted
Rework for comments and API changes.
1 parent ec5f363 commit 496e78e

File tree

2 files changed

+32
-22
lines changed

2 files changed

+32
-22
lines changed

astroquery/alma/core.py

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -212,11 +212,14 @@ def _gen_sql(payload):
212212
return sql + where
213213

214214

215-
#
216-
# Authentication session information for connecting to an OIDC instance. Assumes an OIDC system like Keycloak
217-
# with a preconfigured client called "oidc".
218-
#
219215
class AlmaAuth(BaseQuery):
216+
"""Authentication session information for passing credentials to an OIDC instance
217+
218+
Assumes an OIDC system like Keycloak with a preconfigured client app called "oidc" to validate against.
219+
This does not use Tokens in the traditional OIDC sense, but rather uses the Keycloak specific endpoint
220+
to validate a username and password. Passwords are then kept in a Python keyring.
221+
"""
222+
220223
_CLIENT_ID = 'oidc'
221224
_GRANT_TYPE = 'password'
222225
_INVALID_PASSWORD_MESSAGE = 'Invalid user credentials'
@@ -229,7 +232,12 @@ def __init__(self):
229232
self._auth_hosts = auth_urls
230233
self._auth_host = None
231234

232-
def set_auth_hosts(self, auth_hosts):
235+
@property
236+
def auth_hosts(self):
237+
return self._auth_hosts
238+
239+
@auth_hosts.setter
240+
def auth_hosts(self, auth_hosts):
233241
"""
234242
Set the available hosts to check for login endpoints.
235243
@@ -239,10 +247,12 @@ def set_auth_hosts(self, auth_hosts):
239247
Available hosts name. Checking each one until one returns a 200 for
240248
the well-known endpoint.
241249
"""
242-
self._auth_hosts = auth_hosts
250+
if auth_hosts is None:
251+
raise LoginError('Valid authentication hosts cannot be None')
252+
else:
253+
self._auth_hosts = auth_hosts
243254

244-
@property
245-
def host(self):
255+
def get_valid_host(self):
246256
if self._auth_host is None:
247257
for auth_url in self._auth_hosts:
248258
# set session cookies (they do not get set otherwise)
@@ -277,7 +287,7 @@ def login(self, username, password):
277287
'client_id': self._CLIENT_ID
278288
}
279289

280-
login_url = f'https://{self.host}{self._LOGIN_ENDPOINT}'
290+
login_url = f'https://{self.get_valid_host()}{self._LOGIN_ENDPOINT}'
281291
log.info(f'Authenticating {username} on {login_url}.')
282292
login_response = self._request('POST', login_url, data=data, cache=False)
283293
json_auth = login_response.json()
@@ -287,12 +297,12 @@ def login(self, username, password):
287297
error_message = json_auth['error_description']
288298
if self._INVALID_PASSWORD_MESSAGE not in error_message:
289299
raise LoginError("Could not log in to ALMA authorization portal: "
290-
f"{self.host} Message from server: {error_message}")
300+
f"{self.get_valid_host()} Message from server: {error_message}")
291301
else:
292-
log.error(error_message)
302+
raise LoginError(error_message)
293303
elif 'access_token' not in json_auth:
294304
raise LoginError("Could not log in to any of the known ALMA authorization portals: \n"
295-
f"No error from server, but missing access token from host: {self.host}")
305+
f"No error from server, but missing access token from host: {self.get_valid_host()}")
296306
else:
297307
log.info(f'Successfully logged in to {self._auth_host}')
298308

@@ -965,7 +975,7 @@ def _get_auth_info(self, username, *, store_password=False,
965975
else:
966976
username = self.USERNAME
967977

968-
auth_url = self.auth.host
978+
auth_url = self.auth.get_valid_host()
969979

970980
# Get password from keyring or prompt
971981
password, password_from_keyring = self._get_password(
@@ -995,7 +1005,7 @@ def _login(self, username=None, store_password=False,
9951005
on the keyring. Default is False.
9961006
"""
9971007

998-
self.auth.set_auth_hosts(auth_urls)
1008+
self.auth.auth_hosts = auth_urls
9991009

10001010
username, password = self._get_auth_info(username=username,
10011011
store_password=store_password,

astroquery/alma/tests/test_alma_auth.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ def _requests_mock_ok(method, url, **kwargs):
1313
return response
1414

1515
test_subject = AlmaAuth()
16-
test_subject.set_auth_hosts(['almaexample.com'])
16+
test_subject.auth_hosts = ['almaexample.com']
1717
test_subject._request = Mock(side_effect=_requests_mock_ok)
18-
assert test_subject.host == 'almaexample.com'
18+
assert test_subject.get_valid_host() == 'almaexample.com'
1919

2020

2121
def test_host_default():
@@ -26,7 +26,7 @@ def _requests_mock_ok(method, url, **kwargs):
2626

2727
test_subject = AlmaAuth()
2828
test_subject._request = Mock(side_effect=_requests_mock_ok)
29-
assert test_subject.host == 'asa.alma.cl'
29+
assert test_subject.get_valid_host() == 'asa.alma.cl'
3030

3131

3232
def test_host_err():
@@ -36,10 +36,10 @@ def _requests_mock_err(method, url, **kwargs):
3636
return response
3737

3838
test_subject = AlmaAuth()
39-
test_subject.set_auth_hosts(['almaexample.com'])
39+
test_subject.auth_hosts = ['almaexample.com']
4040
test_subject._request = Mock(side_effect=_requests_mock_err)
4141
with pytest.raises(LoginError):
42-
test_subject.host
42+
test_subject.get_valid_host()
4343

4444

4545
def test_login_bad_error():
@@ -58,7 +58,7 @@ def _requests_mock_err(method, url, **kwargs):
5858
return response
5959

6060
test_subject = AlmaAuth()
61-
test_subject.set_auth_hosts(['almaexample.com'])
61+
test_subject.auth_hosts = ['almaexample.com']
6262
test_subject._request = Mock(side_effect=_requests_mock_err)
6363
with pytest.raises(LoginError) as e:
6464
test_subject.login('TESTUSER', 'TESTPASS')
@@ -80,7 +80,7 @@ def _requests_mock_err(method, url, **kwargs):
8080
return response
8181

8282
test_subject = AlmaAuth()
83-
test_subject.set_auth_hosts(['almaexample.com'])
83+
test_subject.auth_hosts = ['almaexample.com']
8484
test_subject._request = Mock(side_effect=_requests_mock_err)
8585
with pytest.raises(LoginError) as e:
8686
test_subject.login('TESTUSER', 'TESTPASS')
@@ -104,6 +104,6 @@ def _requests_mock_good(method, url, **kwargs):
104104
return response
105105

106106
test_subject = AlmaAuth()
107-
test_subject.set_auth_hosts(['almaexample.com'])
107+
test_subject.auth_hosts = ['almaexample.com']
108108
test_subject._request = Mock(side_effect=_requests_mock_good)
109109
test_subject.login('TESTUSER', 'TESTPASS')

0 commit comments

Comments
 (0)