diff --git a/.mailmap b/.mailmap index 9af20e3937..d6d3dd8d98 100644 --- a/.mailmap +++ b/.mailmap @@ -28,6 +28,7 @@ Hans Moritz Guenter Henrik Norman Henrik Norman Jaladh Singhal +James Dempsey Javier Ballester Javier Duran Javier Duran diff --git a/CHANGES.rst b/CHANGES.rst index d2fc3fb30d..ccc391f5d3 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -23,6 +23,9 @@ casda - Add the ability to produce 2D and 3D cutouts from ASKAP images and cubes. [#2366] +- Use the standard ``login`` method for authenticating, which supports the system + keyring [#2386] + jplsbdb ^^^^^^^ diff --git a/astroquery/casda/__init__.py b/astroquery/casda/__init__.py index 0172f44915..f6ac01ca23 100644 --- a/astroquery/casda/__init__.py +++ b/astroquery/casda/__init__.py @@ -11,7 +11,7 @@ class Conf(_config.ConfigNamespace): server = _config.ConfigItem( ['https://casda.csiro.au/casda_vo_tools/sia2/query'], - 'Name of the CASDA SIA server to use.' + 'Address of the CASDA SIA server to use.' ) timeout = _config.ConfigItem( 30, @@ -25,6 +25,14 @@ class Conf(_config.ConfigNamespace): ['https://casda.csiro.au/casda_data_access/'], 'Address of the CASDA SODA server' ) + login_url = _config.ConfigItem( + ['https://data.csiro.au/casda_vo_proxy/vo/tap/availability'], + 'Address for test logins' + ) + username = _config.ConfigItem( + '', + 'Optional default username for CASDA archive.' + ) conf = Conf() diff --git a/astroquery/casda/core.py b/astroquery/casda/core.py index 6d9b074f9c..c92b369bc2 100644 --- a/astroquery/casda/core.py +++ b/astroquery/casda/core.py @@ -7,6 +7,7 @@ import time from xml.etree import ElementTree from datetime import datetime, timezone +import keyring # 2. third party imports import astropy.units as u @@ -17,10 +18,7 @@ import numpy as np # 3. local imports - use relative imports -# commonly required local imports shown below as example -# all Query classes should inherit from BaseQuery. -from ..query import BaseQuery -# has common functions required by most modules +from ..query import QueryWithLogin from ..utils import commons # prepend_docstr is a way to copy docstrings between methods from ..utils import prepend_docstr_nosections @@ -28,13 +26,14 @@ from ..utils import async_to_sync # import configurable items declared in __init__.py from . import conf +from ..exceptions import LoginError # export all the public classes and methods __all__ = ['Casda', 'CasdaClass'] @async_to_sync -class CasdaClass(BaseQuery): +class CasdaClass(QueryWithLogin): """ Class for accessing ASKAP data through the CSIRO ASKAP Science Data Archive (CASDA). Typical usage: @@ -46,18 +45,62 @@ class CasdaClass(BaseQuery): URL = conf.server TIMEOUT = conf.timeout POLL_INTERVAL = conf.poll_interval + USERNAME = conf.username _soda_base_url = conf.soda_base_url + _login_url = conf.login_url _uws_ns = {'uws': 'http://www.ivoa.net/xml/UWS/v1.0'} - def __init__(self, user=None, password=None): + def __init__(self): super().__init__() - if user is None: - self._authenticated = False + + def _login(self, *, username=None, store_password=False, + reenter_password=False): + """ + login to non-public data as a known user + + Parameters + ---------- + username : str, optional + Username to the CASDA archive, uses ATNF OPAL credentials. If not given, it should be + specified in the config file. + store_password : bool, optional + Stores the password securely in your keyring. Default is False. + reenter_password : bool, optional + Asks for the password even if it is already stored in the + keyring. This is the way to overwrite an already stored passwork + on the keyring. Default is False. + """ + + if username is None: + if not self.USERNAME: + raise LoginError("If you do not pass a username to login(), " + "you should configure a default one!") + else: + username = self.USERNAME + + # Get password from keyring or prompt + password, password_from_keyring = self._get_password( + "astroquery:casda.csiro.au", username, reenter=reenter_password) + + # Login to CASDA to test credentals + log.info("Authenticating {0} on CASDA ...".format(username)) + auth = (username, password) + login_response = self._request("GET", self._login_url, auth=auth, + timeout=self.TIMEOUT, cache=False) + authenticated = login_response.status_code == 200 + + if authenticated: + log.info("Authentication successful!") + self.USERNAME = username + self._auth = (username, password) + + # When authenticated, save password in keyring if needed + if password_from_keyring is None and store_password: + keyring.set_password("astroquery:casda.csiro.au", username, password) else: - self._authenticated = True - # self._user = user - # self._password = password - self._auth = (user, password) + log.exception("Authentication failed") + + return authenticated def query_region_async(self, coordinates, radius=1*u.arcmin, height=None, width=None, get_query_payload=False, cache=True): @@ -274,7 +317,7 @@ def stage_data(self, table, verbose=False): return self._complete_job(job_url, verbose) - def cutout(self, table, *, coordinates=None, radius=None, height=None, + def cutout(self, table, *, coordinates=None, radius=1*u.arcmin, height=None, width=None, band=None, channel=None, verbose=False): """ Produce a cutout from each selected file. All requests for data must use authentication. If you have access to diff --git a/astroquery/casda/tests/data/availability.xml b/astroquery/casda/tests/data/availability.xml new file mode 100644 index 0000000000..d518b2bdfe --- /dev/null +++ b/astroquery/casda/tests/data/availability.xml @@ -0,0 +1,6 @@ + + + true + 2022-04-07T11:11:40.006Z + + \ No newline at end of file diff --git a/astroquery/casda/tests/test_casda.py b/astroquery/casda/tests/test_casda.py index 3ac298f1dc..80be64f286 100644 --- a/astroquery/casda/tests/test_casda.py +++ b/astroquery/casda/tests/test_casda.py @@ -2,10 +2,10 @@ # Licensed under a 3-clause BSD style license - see LICENSE.rst - import pytest import requests import os +import keyring from astropy.coordinates import SkyCoord import astropy.units as u @@ -15,6 +15,7 @@ import numpy as np from astroquery.casda import Casda +from astroquery.exceptions import LoginError try: from unittest.mock import Mock, patch, MagicMock @@ -23,7 +24,11 @@ DATA_FILES = {'CIRCLE': 'cone.xml', 'RANGE': 'box.xml', 'DATALINK': 'datalink.xml', 'RUN_JOB': 'run_job.xml', 'COMPLETED_JOB': 'completed_job.xml', 'DATALINK_NOACCESS': 'datalink_noaccess.xml', - 'cutout_CIRCLE_333.9092_-45.8418_0.5000': 'cutout_333.9092_-45.8418_0.5000.xml'} + 'cutout_CIRCLE_333.9092_-45.8418_0.5000': 'cutout_333.9092_-45.8418_0.5000.xml', + 'AVAILABILITY': 'availability.xml'} + +USERNAME = 'user' +PASSWORD = 'password' class MockResponse: @@ -31,6 +36,7 @@ class MockResponse: def __init__(self, content): self.content = content self.text = content.decode() + self.status_code = 200 def raise_for_status(self): return @@ -41,7 +47,7 @@ def get_mockreturn(self, method, url, data=None, timeout=10, log.debug("get_mockreturn url:{} params:{} kwargs:{}".format(url, params, kwargs)) if kwargs and 'auth' in kwargs: auth = kwargs['auth'] - if auth and (auth[0] != 'user' or auth[1] != 'password'): + if auth and (auth[0] != USERNAME or auth[1] != PASSWORD): log.debug("Rejecting credentials") return create_auth_failure_response() @@ -71,6 +77,8 @@ def get_mockreturn(self, method, url, data=None, timeout=10, key = 'DATALINK' else: key = 'DATALINK_NOACCESS' + elif str(url) == 'https://data.csiro.au/casda_vo_proxy/vo/tap/availability': + key = 'AVAILABILITY' else: key = params['POS'].split()[0] if params['POS'] else None filename = data_path(DATA_FILES[key]) @@ -117,6 +125,39 @@ def isclose(value1, value2, abs_tol=1e-09): return abs(value1 - value2) < abs_tol +def fake_login(casda, username, password): + casda.USERNAME = username + casda._auth = (username, password) + casda._authenticated = True + + +def test_login_no_default_user(): + casda = Casda() + assert casda._authenticated is False + assert casda.USERNAME == '' + + with pytest.raises(LoginError, match=r"If you do not pass a username to login\(\),") as excinfo: + Casda.login() + + assert casda._authenticated is False + assert casda.USERNAME == '' + assert hasattr(casda, '_auth') is False + + +@pytest.mark.skip('No keyring backend on the CI server') +def test_login_keyring(patch_get): + casda = Casda() + assert casda._authenticated is False + assert casda.USERNAME == '' + keyring.set_password("astroquery:casda.csiro.au", USERNAME, PASSWORD) + + casda.login(username=USERNAME) + keyring.delete_password("astroquery:casda.csiro.au", USERNAME) + assert casda._authenticated is True + assert casda.USERNAME == USERNAME + assert casda._auth == (USERNAME, PASSWORD) + + def test_query_region_text_radius(patch_get): ra = 333.9092 dec = -45.8418 @@ -243,15 +284,14 @@ def test_filter_out_unreleased(): def test_stage_data_unauthorised(patch_get): table = Table() - with pytest.raises(ValueError) as excinfo: + with pytest.raises(ValueError, match=r"Credentials must be supplied") as excinfo: Casda.stage_data(table) - assert "Credentials must be supplied" in str(excinfo.value) - def test_stage_data_empty(patch_get): table = Table() - casda = Casda('user', 'password') + casda = Casda() + fake_login(casda, USERNAME, PASSWORD) urls = casda.stage_data(table) assert urls == [] @@ -261,7 +301,12 @@ def test_stage_data_invalid_credentials(patch_get): access_urls = [prefix + 'cube-220'] table = Table([Column(data=access_urls, name='access_url')]) - casda = Casda('user', 'notthepassword') + casda = Casda() + # Update the casda object to indicate that it has been authenticated + casda.USERNAME = USERNAME + casda._auth = (USERNAME, 'notthepassword') + casda._authenticated = True + with pytest.raises(requests.exceptions.HTTPError) as excinfo: casda.stage_data(table) @@ -270,20 +315,20 @@ def test_stage_data_no_link(patch_get): prefix = 'https://somewhere/casda/datalink/links?' access_urls = [prefix + 'cube-240'] table = Table([Column(data=access_urls, name='access_url')]) - casda = Casda('user', 'password') + casda = Casda() + fake_login(casda, USERNAME, PASSWORD) casda.POLL_INTERVAL = 1 - with pytest.raises(ValueError) as excinfo: + with pytest.raises(ValueError, match=r"You do not have access to any of the requested data files\.") as excinfo: casda.stage_data(table) - assert "You do not have access to any of the requested data files." in str(excinfo.value) - def test_stage_data(patch_get): prefix = 'https://somewhere/casda/datalink/links?' access_urls = [prefix + 'cube-244'] table = Table([Column(data=access_urls, name='access_url')]) - casda = Casda('user', 'password') + casda = Casda() + fake_login(casda, USERNAME, PASSWORD) casda.POLL_INTERVAL = 1 urls = casda.stage_data(table, verbose=True) assert urls == ['http://casda.csiro.au/download/web/111-000-111-000/askap_img.fits.checksum', @@ -299,7 +344,8 @@ def test_cutout(patch_get): radius = 30*u.arcmin centre = SkyCoord(ra, dec) - casda = Casda('user', 'password') + casda = Casda() + fake_login(casda, USERNAME, PASSWORD) casda.POLL_INTERVAL = 1 urls = casda.cutout(table, coordinates=centre, radius=radius, verbose=True) assert urls == ['http://casda.csiro.au/download/web/111-000-111-000/cutout.fits.checksum', @@ -315,11 +361,12 @@ def test_cutout_no_args(patch_get): radius = 30*u.arcmin centre = SkyCoord(ra, dec) - casda = Casda('user', 'password') + casda = Casda() + fake_login(casda, USERNAME, PASSWORD) casda.POLL_INTERVAL = 1 - with pytest.raises(ValueError) as excinfo: + with pytest.raises(ValueError, + match=r"Please provide cutout parameters such as coordinates, band or channel\.") as excinfo: casda.cutout(table) - assert "Please provide cutout parameters such as coordinates, band or channel." in str(excinfo.value) def test_cutout_unauthorised(patch_get): @@ -331,20 +378,23 @@ def test_cutout_unauthorised(patch_get): radius = 30*u.arcmin centre = SkyCoord(ra, dec) - with pytest.raises(ValueError) as excinfo: + with pytest.raises(ValueError, match=r"Credentials must be supplied to download CASDA image data") as excinfo: Casda.cutout(table, coordinates=centre, radius=radius, verbose=True) - assert "Credentials must be supplied to download CASDA image data" in str(excinfo.value) -def test_cutout_no_table(): - casda = Casda('user', 'password') +def test_cutout_no_table(patch_get): + casda = Casda() + fake_login(casda, USERNAME, PASSWORD) + casda.POLL_INTERVAL = 1 result = casda.cutout(None) assert result == [] -def test_args_to_payload_band(): - casda = Casda('user', 'password') +def test_args_to_payload_band(patch_get): + casda = Casda() + fake_login(casda, USERNAME, PASSWORD) + payload = casda._args_to_payload(band=(0.195*u.m, 0.215*u.m)) assert payload['BAND'] == '0.195 0.215' assert list(payload.keys()) == ['BAND'] @@ -382,8 +432,9 @@ def test_args_to_payload_band(): assert list(payload.keys()) == ['BAND'] -def test_args_to_payload_band_invalid(): - casda = Casda('user', 'password') +def test_args_to_payload_band_invalid(patch_get): + casda = Casda() + fake_login(casda, USERNAME, PASSWORD) with pytest.raises(ValueError) as excinfo: casda._args_to_payload(band='foo') @@ -410,8 +461,10 @@ def test_args_to_payload_band_invalid(): assert "Either 'channel' or 'band' values may be provided but not both." in str(excinfo.value) -def test_args_to_payload_channel(): - casda = Casda('user', 'password') +def test_args_to_payload_channel(patch_get): + casda = Casda() + fake_login(casda, USERNAME, PASSWORD) + payload = casda._args_to_payload(channel=(0, 30)) assert payload['CHANNEL'] == '0 30' assert list(payload.keys()) == ['CHANNEL'] @@ -425,8 +478,9 @@ def test_args_to_payload_channel(): assert list(payload.keys()) == ['CHANNEL'] -def test_args_to_payload_channel_invalid(): - casda = Casda('user', 'password') +def test_args_to_payload_channel_invalid(patch_get): + casda = Casda() + fake_login(casda, USERNAME, PASSWORD) with pytest.raises(ValueError) as excinfo: casda._args_to_payload(channel='one') @@ -445,8 +499,10 @@ def test_args_to_payload_channel_invalid(): assert "The 'channel' value must be a list of 2 integer values." in str(excinfo.value) -def test_args_to_payload_coordinates(): - casda = Casda('user', 'password') +def test_args_to_payload_coordinates(patch_get): + casda = Casda() + fake_login(casda, USERNAME, PASSWORD) + cutout_coords = SkyCoord(ra=345.245*u.degree, dec=-32.125*u.degree, frame='icrs') payload = casda._args_to_payload(coordinates=cutout_coords) assert payload['POS'].startswith('CIRCLE 345') @@ -482,8 +538,9 @@ def test_args_to_payload_coordinates(): assert list(payload.keys()) == ['POS'] -def test_args_to_payload_combined(): - casda = Casda('user', 'password') +def test_args_to_payload_combined(patch_get): + casda = Casda() + fake_login(casda, USERNAME, PASSWORD) cutout_coords = SkyCoord(ra=187.5*u.degree, dec=-60.0*u.degree, frame='icrs') payload = casda._args_to_payload(coordinates=cutout_coords, channel=(17, 23)) assert payload['POS'].startswith('CIRCLE 187') @@ -501,7 +558,8 @@ def test_download_file(patch_get): urls = ['https://ingest.pawsey.org/bucket_name/path/askap_img.fits?security=stuff', 'http://casda.csiro.au/download/web/111-000-111-000/askap_img.fits.checksum', 'https://ingest.pawsey.org.au/casda-prd-as110-01/dc52217/primary_images/RACS-DR1_0000%2B18A.fits?security=stuff'] - casda = Casda('user', 'password') + casda = Casda() + fake_login(casda, USERNAME, PASSWORD) # skip the actual downloading of the file download_mock = MagicMock() diff --git a/astroquery/casda/tests/test_casda_remote.py b/astroquery/casda/tests/test_casda_remote.py index 2568d9eadb..3c2b20fc39 100644 --- a/astroquery/casda/tests/test_casda_remote.py +++ b/astroquery/casda/tests/test_casda_remote.py @@ -2,6 +2,7 @@ # Licensed under a 3-clause BSD style license - see LICENSE.rst +import keyring import math import os import pytest @@ -39,34 +40,43 @@ def test_query_region_text_radius(self): for key in ('dataproduct_type', 'obs_id', 'access_url', 'access_format', 'obs_release_date'): assert key in responses.keys() - @pytest.mark.skipif(('CASDA_USER' not in os.environ or - 'CASDA_PASSWD' not in os.environ), + @pytest.fixture + def cached_credentials(self): + if 'CASDA_USER' in os.environ and 'CASDA_PASSWD' not in os.environ: + keyring.set_password("astroquery:casda.csiro.au", os.environ['CASDA_USER'], os.environ['CASDA_PASSWD']) + yield + keyring.delete_password("astroquery:casda.csiro.au", os.environ['CASDA_USER']) + else: + yield + + @pytest.mark.skipif(('CASDA_USER' not in os.environ), reason='Requires real CASDA user/password (CASDA_USER ' - 'and CASDA_PASSWD environment variables)') - def test_stage_data(self): + 'and CASDA_PASSWD environment variables or password in keyring)') + def test_stage_data(self, cached_credentials): prefix = 'https://data.csiro.au/casda_vo_proxy/vo/datalink/links?ID=' access_urls = [prefix + 'cube-1262'] table = Table([Column(data=access_urls, name='access_url')]) - casda = Casda(os.environ['CASDA_USER'], os.environ['CASDA_PASSWD']) + casda = Casda() + casda.login(username=os.environ['CASDA_USER']) casda.POLL_INTERVAL = 3 urls = casda.stage_data(table) - assert str(urls[0]).endswith('image_cube_g300to310.q.fits') + assert 'g300to310.q.fits' in str(urls[0]) assert str(urls[1]).endswith('image_cube_g300to310.q.fits.checksum') assert len(urls) == 2 - @pytest.mark.skipif(('CASDA_USER' not in os.environ or - 'CASDA_PASSWD' not in os.environ), + @pytest.mark.skipif(('CASDA_USER' not in os.environ), reason='Requires real CASDA user/password (CASDA_USER ' - 'and CASDA_PASSWD environment variables)') - def test_cutout(self): + 'and CASDA_PASSWD environment variables or password in keyring)') + def test_cutout(self, cached_credentials): prefix = 'https://data.csiro.au/casda_vo_proxy/vo/datalink/links?ID=' access_urls = [prefix + 'cube-44705'] table = Table([Column(data=access_urls, name='access_url')]) - casda = Casda(os.environ['CASDA_USER'], os.environ['CASDA_PASSWD']) + casda = Casda() + casda.login(username=os.environ['CASDA_USER']) casda.POLL_INTERVAL = 3 pos = SkyCoord(196.49583333*u.deg, -62.7*u.deg) - urls = casda.cutout(table, pos, radius=15*u.arcmin) + urls = casda.cutout(table, coordinates=pos, radius=15*u.arcmin) # URLs may come back in any order for url in urls: diff --git a/docs/casda/casda.rst b/docs/casda/casda.rst index c29ed9ef09..bf9ae9c7ac 100644 --- a/docs/casda/casda.rst +++ b/docs/casda/casda.rst @@ -71,15 +71,14 @@ date. To change user-registration details, or to request a new OPAL password, use the link to 'Log in or reset password'. To use download tasks, you should create an instance of the ``Casda`` class -with a username and password. e.g.: +and call the :meth:`~astroquery.casda.CasdaClass.login` method with a username. The password will either be taken +from the keyring, or if in an interactive environment then it will be requested. e.g.: .. doctest-skip:: >>> from astroquery.casda import Casda - >>> import getpass - >>> username = 'email@somewhere.edu.au' - >>> password = getpass.getpass(str("Enter your OPAL password: ")) - >>> casda = Casda(username, password) + >>> casda = Casda() + >>> casda.login(username='email@somewhere.edu.au') Data Access @@ -103,11 +102,9 @@ taken in scheduling block 2338 is shown below: >>> from astropy import coordinates, units as u, wcs >>> from astroquery.casda import Casda - >>> import getpass >>> centre = coordinates.SkyCoord.from_name('NGC 7232') - >>> username = 'email@somewhere.edu.au' - >>> password = getpass.getpass(str("Enter your OPAL password: ")) - >>> casda = Casda(username, password) + >>> casda = Casda() + >>> casda.login(username='email@somewhere.edu.au') >>> result = Casda.query_region(centre, radius=30*u.arcmin) >>> public_data = Casda.filter_out_unreleased(result) >>> subset = public_data[(public_data['dataproduct_subtype']=='cont.restored.t0') & (public_data['obs_id']=='2338')] @@ -138,11 +135,9 @@ below: >>> from astropy import coordinates, units as u, wcs >>> from astroquery.casda import Casda - >>> import getpass >>> centre = coordinates.SkyCoord.from_name('2MASX J08161181-7039447') - >>> username = 'email@somewhere.edu.au' - >>> password = getpass.getpass(str("Enter your OPAL password: ")) - >>> casda = Casda(username, password) + >>> casda = Casda() + >>> casda.login(username='email@somewhere.edu.au') >>> result = Casda.query_region(centre, radius=30*u.arcmin) >>> public_data = Casda.filter_out_unreleased(result) >>> subset = public_data[((public_data['obs_collection'] == 'The Rapid ASKAP Continuum Survey') & # @@ -159,11 +154,9 @@ is shown below: >>> from astropy import coordinates, units as u, wcs >>> from astroquery.casda import Casda - >>> import getpass >>> centre = coordinates.SkyCoord.from_name('NGC 1371') - >>> username = 'email@somewhere.edu.au' - >>> password = getpass.getpass(str("Enter your OPAL password: ")) - >>> casda = Casda(username, password) + >>> casda = Casda() + >>> casda.login(username='email@somewhere.edu.au') >>> result = Casda.query_region(centre, radius=30*u.arcmin) >>> public_data = Casda.filter_out_unreleased(result) >>> eridanus_cube = public_data[public_data['filename'] == 'Eridanus_full_image_V3.fits']