Resolves #135

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
 

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.')
 

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

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=<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}')