diff --git a/CHANGELOG.md b/CHANGELOG.md index d5ff3a9e..cd3b9fe5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ This version improves behaviour of loading revisions and loading datasets from l - default_revisions_only parameter in One.list_datasets filters non-default datasets - permit data frame input to One.load_datasets and load precise relative paths provided (instead of default revisions) - redundent session_path column has been dropped from the datasets cache table +- bugfix in one.params.setup: suggest previous cache dir if available instead of always the default +- bugfix in one.params.setup: remove all extrenuous parameters (i.e. TOKEN) when running setup in silent mode +- warn user to reauthenticate when password is None in silent mode +- always force authentication when password passed, even when token cached ### Added diff --git a/one/params.py b/one/params.py index 47409f9e..6de042cc 100644 --- a/one/params.py +++ b/one/params.py @@ -158,7 +158,8 @@ def setup(client=None, silent=False, make_default=None, username=None, cache_dir # Prompt for cache directory (default may have changed after prompt) client_key = _key_from_url(par.ALYX_URL) - cache_dir = cache_dir or Path(CACHE_DIR_DEFAULT, client_key) + def_cache_dir = cache_map.CLIENT_MAP.get(client_key) or Path(CACHE_DIR_DEFAULT, client_key) + cache_dir = cache_dir or def_cache_dir prompt = f'Enter the location of the download cache, current value is ["{cache_dir}"]:' cache_dir = input(prompt) or cache_dir @@ -185,7 +186,9 @@ def setup(client=None, silent=False, make_default=None, username=None, cache_dir # Precedence: user provided cache_dir; previously defined; the default location default_cache_dir = Path(CACHE_DIR_DEFAULT, client_key) cache_dir = cache_dir or cache_map.CLIENT_MAP.get(client_key, default_cache_dir) - par = par_current + # Use current params but drop any extras (such as the TOKEN or ALYX_PWD field) + keep_keys = par_default.as_dict().keys() + par = iopar.from_dict({k: v for k, v in par_current.as_dict().items() if k in keep_keys}) if any(v for k, v in cache_map.CLIENT_MAP.items() if k != client_key and v == cache_dir): warnings.warn('Warning: the directory provided is already a cache for another URL.') diff --git a/one/tests/test_alyxclient.py b/one/tests/test_alyxclient.py index 728c033c..3a7e9e30 100644 --- a/one/tests/test_alyxclient.py +++ b/one/tests/test_alyxclient.py @@ -51,10 +51,17 @@ def test_authentication(self): ac.authenticate() mock_input.assert_not_called() self.assertTrue(ac.is_logged_in) + + # When password is None and in silent mode, there should be a warning + # followed by a failed login attempt + ac._par = ac._par.set('ALYX_PWD', None) + ac.logout() + with self.assertWarns(UserWarning), self.assertRaises(requests.HTTPError): + self.ac.authenticate(password=None) + # Test using input args ac._par = iopar.from_dict({k: v for k, v in ac._par.as_dict().items() if k not in login_keys}) - ac.logout() with mock.patch('builtins.input') as mock_input: ac.authenticate(TEST_DB_2['username'], TEST_DB_2['password'], cache_token=False) mock_input.assert_not_called() @@ -76,6 +83,16 @@ def test_authentication(self): with mock.patch('one.webclient.getpass', return_value=TEST_DB_2['password']) as mock_pwd: ac.authenticate(cache_token=True, force=True) mock_pwd.assert_called() + # If a password is passed, should always force re-authentication + rep = requests.Response() + rep.status_code = 200 + rep.json = lambda **_: {'token': 'abc'} + assert self.ac.is_logged_in + with mock.patch('one.webclient.requests.post', return_value=rep) as m: + self.ac.authenticate(password='foo', force=False) + expected = {'username': TEST_DB_2['username'], 'password': 'foo'} + m.assert_called_once_with(TEST_DB_2['base_url'] + '/auth-token', data=expected) + # Check non-silent double logout ac.logout() ac.logout() # Shouldn't complain diff --git a/one/webclient.py b/one/webclient.py index 0782122c..fda446d0 100644 --- a/one/webclient.py +++ b/one/webclient.py @@ -642,6 +642,11 @@ def authenticate(self, username=None, password=None, cache_token=True, force=Fal if username is None and not self.silent: username = input('Enter Alyx username:') + # If user passes in a password, force re-authentication even if token cached + if password is not None: + if not force: + _logger.debug('Forcing token request with provided password') + force = True # Check if token cached if not force and getattr(self._par, 'TOKEN', False) and username in self._par.TOKEN: self._token = self._par.TOKEN[username] @@ -654,8 +659,17 @@ def authenticate(self, username=None, password=None, cache_token=True, force=Fal # Get password if password is None: password = getattr(self._par, 'ALYX_PWD', None) - if password is None and not self.silent: - password = getpass(f'Enter Alyx password for "{username}":') + if password is None: + if self.silent: + warnings.warn( + 'No password or cached token in silent mode. ' + 'Please run the following to re-authenticate:\n\t' + 'AlyxClient(silent=False).authenticate' + '(username=, force=True)', UserWarning) + else: + password = getpass(f'Enter Alyx password for "{username}":') + # Remove previous token + self._clear_token(username) try: credentials = {'username': username, 'password': password} rep = requests.post(self.base_url + '/auth-token', data=credentials) @@ -692,14 +706,12 @@ def authenticate(self, username=None, password=None, cache_token=True, force=Fal if not self.silent: print(f'Connected to {self.base_url} as user "{self.user}"') - def logout(self): - """Log out from Alyx. - Deletes the cached authentication token for the currently logged-in user. + def _clear_token(self, username): + """Remove auth token from client params. + + Deletes the cached authentication token for a given user. """ - if not self.is_logged_in: - return par = one.params.get(client=self.base_url, silent=True) - username = self.user # Remove token from cache if getattr(par, 'TOKEN', False) and username in par.TOKEN: del par.TOKEN[username] @@ -708,10 +720,20 @@ def logout(self): if getattr(self._par, 'TOKEN', False) and username in self._par.TOKEN: del self._par.TOKEN[username] # Remove token from object - self.user = None self._token = None if self._headers and 'Authorization' in self._headers: del self._headers['Authorization'] + + def logout(self): + """Log out from Alyx. + + Deletes the cached authentication token for the currently logged-in user + and clears the REST cache. + """ + if not self.is_logged_in: + return + self._clear_token(username := self.user) + self.user = None self.clear_rest_cache() if not self.silent: print(f'{username} logged out from {self.base_url}')