From c4ea75e4e6bdc51c2c9f81299f2219af3f00efb9 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Wed, 23 Sep 2026 13:05:59 -0500 Subject: [PATCH 1/3] Resolve issue with displaying a null exception when the user is not found. It now simply shows invalid credentials for user... --- .../logr/rest/routes/AuthenticationRoute.java | 4 +- .../rest/routes/AuthenticationRouteTest.java | 39 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 src/java/LogrPortal/test/gov/anl/aps/logr/rest/routes/AuthenticationRouteTest.java diff --git a/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/routes/AuthenticationRoute.java b/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/routes/AuthenticationRoute.java index 343324b58..df142dbc0 100644 --- a/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/routes/AuthenticationRoute.java +++ b/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/routes/AuthenticationRoute.java @@ -68,8 +68,8 @@ public class AuthenticationRoute extends BaseRoute { public Response authenticateUser(@FormParam("username") String username, @FormParam("password") String password) throws AuthenticationError { LOGGER.debug("Authenticating user: " + username); - UserInfo userInfo = userFacade.findByUsername(username); - boolean authenticated = LoginController.validateCredentials(userInfo, password); + UserInfo userInfo = userFacade.findByUsername(username); + boolean authenticated = userInfo != null && LoginController.validateCredentials(userInfo, password); if (authenticated) { UserSessionKeeper usk = UserSessionKeeper.getInstance(); diff --git a/src/java/LogrPortal/test/gov/anl/aps/logr/rest/routes/AuthenticationRouteTest.java b/src/java/LogrPortal/test/gov/anl/aps/logr/rest/routes/AuthenticationRouteTest.java new file mode 100644 index 000000000..b2b6135b6 --- /dev/null +++ b/src/java/LogrPortal/test/gov/anl/aps/logr/rest/routes/AuthenticationRouteTest.java @@ -0,0 +1,39 @@ +/* + * Copyright (c) UChicago Argonne, LLC. All rights reserved. + * See LICENSE file. + */ +package gov.anl.aps.logr.rest.routes; + +import gov.anl.aps.logr.common.exceptions.AuthenticationError; +import gov.anl.aps.logr.portal.model.db.beans.UserInfoFacade; +import gov.anl.aps.logr.portal.model.db.entities.UserInfo; +import java.lang.reflect.Field; + +/** Regression test for REST login by an unknown user. */ +public class AuthenticationRouteTest { + + public static void main(String[] args) throws Exception { + AuthenticationRoute route = new AuthenticationRoute(); + Field userFacade = AuthenticationRoute.class.getDeclaredField("userFacade"); + userFacade.setAccessible(true); + userFacade.set(route, new UnknownUserFacade()); + + try { + route.authenticateUser("unknown-user", "password"); + throw new AssertionError("Expected authentication to fail"); + } catch (AuthenticationError ex) { + if (!"Could not verify username or password.".equals(ex.getMessage())) { + throw new AssertionError("Unexpected authentication error: " + ex.getMessage()); + } + } + + System.out.println("PASS unknown user returns authentication error"); + } + + private static class UnknownUserFacade extends UserInfoFacade { + @Override + public UserInfo findByUsername(String username) { + return null; + } + } +} From f5bca9659e07befbdef3b217861d8833125e0a79 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Wed, 23 Sep 2026 13:51:45 -0500 Subject: [PATCH 2/3] Add a logout function --- tools/developer_tools/bely-cli/README.md | 7 ++++ tools/developer_tools/bely-cli/run_test.sh | 1 + .../bely-cli/src/bely_cli/auth.py | 25 ++++++++++++ .../bely-cli/src/bely_cli/cli.py | 8 ++++ .../bely-cli/src/bely_cli/commands.py | 7 ++++ .../bely-cli/test/test_auth.py | 40 +++++++++++++++++++ .../bely-cli/test/test_commands.py | 26 ++++++++++++ 7 files changed, 114 insertions(+) diff --git a/tools/developer_tools/bely-cli/README.md b/tools/developer_tools/bely-cli/README.md index a04a1c03a..febf7cd60 100644 --- a/tools/developer_tools/bely-cli/README.md +++ b/tools/developer_tools/bely-cli/README.md @@ -95,6 +95,13 @@ lookups (listing types, systems, templates, finding documents) do not. On success a token is cached at `~/.config/bely/token` (permissions `0600`) and reused on later runs. Expired or invalid tokens are discarded and you re-authenticate automatically. +To invalidate the current token on the server and remove it locally, run: + +```bash +bely-cli logout +``` + +If no token is cached, the command reports `Not logged in.` and succeeds. The token location can be changed with the `token_path` setting; by default it sits beside the settings file (see [Configuration & environment](#configuration--environment)). diff --git a/tools/developer_tools/bely-cli/run_test.sh b/tools/developer_tools/bely-cli/run_test.sh index 5e18ef6b8..55daa3e80 100755 --- a/tools/developer_tools/bely-cli/run_test.sh +++ b/tools/developer_tools/bely-cli/run_test.sh @@ -17,6 +17,7 @@ $RUNNER python -m unittest # (appended to a leaf command, not at the top level). $RUNNER bely-cli -h > /dev/null $RUNNER bely-cli doc list -h | grep -q -- --format +$RUNNER bely-cli logout -h | grep -q -- --format $RUNNER bely-cli tui lookup -h | grep -q -- --format # Bare `tui` is the one group that carries --limit/--format itself (see CLAUDE.md). $RUNNER bely-cli tui -h | grep -q -- --limit diff --git a/tools/developer_tools/bely-cli/src/bely_cli/auth.py b/tools/developer_tools/bely-cli/src/bely_cli/auth.py index 2cce4ceca..dcef4c889 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/auth.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/auth.py @@ -119,6 +119,31 @@ def authenticated_factory_from_token(): return factory +def logout(): + """Invalidate and remove the cached token. Return False if no token was cached.""" + import belyApi + from BelyApiFactory import BelyApiFactory + + token = load_token() + if not token: + return False + + factory = BelyApiFactory(bely_url=get_host()) + factory.api_client.set_default_header(BelyApiFactory.HEADER_TOKEN_KEY, token) + try: + factory.logout_user() + except belyApi.exceptions.UnauthorizedException: + pass + except Exception as e: + from .common import format_error_message + + raise RuntimeError(f"Logout failed: {format_error_message(e, factory)}") from e + finally: + delete_token() + + return True + + def login(username, password): """Authenticate with credentials, cache the resulting token, and return the factory. diff --git a/tools/developer_tools/bely-cli/src/bely_cli/cli.py b/tools/developer_tools/bely-cli/src/bely_cli/cli.py index 85741ff15..7f9bd7b02 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/cli.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/cli.py @@ -7,6 +7,7 @@ from .commands import ( cmd_new_doc, cmd_list_docs, + cmd_logout, cmd_show_config, cmd_edit_config, cmd_set_config, @@ -56,6 +57,13 @@ def cli(): pass +@cli.command("logout") +@common_options +def logout(output_format): + """Log out and remove the cached authentication token.""" + cmd_logout(fmt=output_format) + + # -- doc -- @cli.group("doc") diff --git a/tools/developer_tools/bely-cli/src/bely_cli/commands.py b/tools/developer_tools/bely-cli/src/bely_cli/commands.py index 0da826a86..2e9e761cc 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/commands.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/commands.py @@ -12,6 +12,13 @@ ENV_VARS = core.ENV_VARS +def cmd_logout(fmt="text"): + """Invalidate the current token and remove it from the local cache.""" + logged_out = auth.logout() + message = "Logged out." if logged_out else "Not logged in." + print_result({"logged_out": logged_out}, message, fmt) + + def cmd_show_config(fmt="text"): """Show current configuration from settings file and environment.""" data = core.collect_config() diff --git a/tools/developer_tools/bely-cli/test/test_auth.py b/tools/developer_tools/bely-cli/test/test_auth.py index 0cc1b0cc6..5cd7243c7 100644 --- a/tools/developer_tools/bely-cli/test/test_auth.py +++ b/tools/developer_tools/bely-cli/test/test_auth.py @@ -44,6 +44,10 @@ def authenticate_user(self, username, password): def get_authenticate_token(self): return self.api_client.default_headers[self.HEADER_TOKEN_KEY] + def logout_user(self): + if self.api_client.default_headers.get(self.HEADER_TOKEN_KEY) != valid_token: + raise _Unauthorized() + return FakeFactory @@ -119,6 +123,42 @@ def test_rejected_token_is_deleted_and_returns_none(self): self.assertIsNone(auth.load_token()) +class LogoutTests(AuthTestCase): + def test_no_cached_token_returns_false(self): + self._install_factory(valid_token="good-token") + + self.assertFalse(auth.logout()) + + def test_valid_cached_token_is_invalidated_and_deleted(self): + self._install_factory(valid_token="good-token") + auth.save_token("good-token") + + self.assertTrue(auth.logout()) + self.assertIsNone(auth.load_token()) + + def test_rejected_cached_token_is_still_deleted(self): + self._install_factory(valid_token="good-token") + auth.save_token("stale-token") + + self.assertTrue(auth.logout()) + self.assertIsNone(auth.load_token()) + + def test_other_failure_still_deletes_local_token(self): + fake_cls = _fake_factory_class(valid_token="good-token") + + class BoomFactory(fake_cls): + def logout_user(self): + raise RuntimeError("network down") + + self._install_factory_class(BoomFactory) + auth.save_token("good-token") + + with self.assertRaisesRegex(RuntimeError, "Logout failed"): + auth.logout() + + self.assertIsNone(auth.load_token()) + + class LoginTests(AuthTestCase): def test_success_caches_token_and_returns_factory(self): self._install_factory(login_ok=True, login_token="fresh-token") diff --git a/tools/developer_tools/bely-cli/test/test_commands.py b/tools/developer_tools/bely-cli/test/test_commands.py index f86fe9a9e..dabbe2c69 100644 --- a/tools/developer_tools/bely-cli/test/test_commands.py +++ b/tools/developer_tools/bely-cli/test/test_commands.py @@ -40,6 +40,32 @@ def add_update_log_entry(self, log_entry): return log_entry +class CmdLogoutTests(unittest.TestCase): + def test_logs_out_current_session(self): + with patch.object(commands.auth, "logout", return_value=True): + buf = io.StringIO() + with redirect_stdout(buf): + commands.cmd_logout() + + self.assertEqual(buf.getvalue(), "Logged out.\n") + + def test_reports_when_not_logged_in(self): + with patch.object(commands.auth, "logout", return_value=False): + buf = io.StringIO() + with redirect_stdout(buf): + commands.cmd_logout() + + self.assertEqual(buf.getvalue(), "Not logged in.\n") + + def test_structured_output_reports_status(self): + with patch.object(commands.auth, "logout", return_value=True): + buf = io.StringIO() + with redirect_stdout(buf): + commands.cmd_logout(fmt="json") + + self.assertEqual(buf.getvalue(), '{"logged_out": true}\n') + + class CmdNewDocTests(unittest.TestCase): def test_creates_doc_and_first_entry_from_file(self): api = FakeApi() From b5fe5f3c2a177a903562f6916d57d7afdc7c1271 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Wed, 23 Sep 2026 13:56:41 -0500 Subject: [PATCH 3/3] Add ability to log out inside the tui --- tools/developer_tools/bely-cli/README.md | 6 ++-- .../bely-cli/src/bely_cli/auth.py | 14 ++++---- .../bely-cli/src/bely_cli/tui/app.py | 22 +++++++++++-- .../bely-cli/src/bely_cli/tui/session.py | 8 +++++ .../bely-cli/test/test_tui_app.py | 32 ++++++++++++++++++- 5 files changed, 70 insertions(+), 12 deletions(-) diff --git a/tools/developer_tools/bely-cli/README.md b/tools/developer_tools/bely-cli/README.md index febf7cd60..45e0af0a3 100644 --- a/tools/developer_tools/bely-cli/README.md +++ b/tools/developer_tools/bely-cli/README.md @@ -190,7 +190,7 @@ equivalent of Home's old menu items, plus what Textual provides by default: |---------|--------| | Configuration | Opens the configuration dialog — equivalent of `config show` / `config set` / `config edit`. | | My documents | Opens a browse starting at your recently modified documents — equivalent of `doc list`. `Esc` pops back to wherever you opened it from. | -| Log in | Authenticate now instead of waiting for the first mutation. | +| Log in / Log out | Authenticate now instead of waiting for the first mutation, or end the current authenticated session and remove its cached token. | | Refresh cache | Discard all cached logbook data so the next view re-fetches from the server. | | Theme | Built-in: change the app's color theme; the choice is saved as the `theme` setting and reused on the next launch. | | Quit | Built-in: exit the app. | @@ -252,7 +252,9 @@ save a config change, the app looks for the token the CLI already caches (see already run an authenticated `bely-cli` command, or a previous `tui` session, you won't be prompted again. Otherwise a login modal appears (username, password, and `Log in`/`Cancel` buttons — `ctrl+s` also submits, `Esc` also cancels); a successful login is cached the same -way the CLI caches it, shared by later `bely-cli` commands and TUI sessions alike. +way the CLI caches it, shared by later `bely-cli` commands and TUI sessions alike. Once +logged in, the command palette offers `Log out`, which invalidates the server session, +removes the cached token, and returns the TUI to its unauthenticated state. #### `bely-cli tui lookup` diff --git a/tools/developer_tools/bely-cli/src/bely_cli/auth.py b/tools/developer_tools/bely-cli/src/bely_cli/auth.py index dcef4c889..c67e58fe0 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/auth.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/auth.py @@ -119,17 +119,17 @@ def authenticated_factory_from_token(): return factory -def logout(): - """Invalidate and remove the cached token. Return False if no token was cached.""" +def logout(factory=None): + """Invalidate and remove the cached token. Return False if no session exists.""" import belyApi from BelyApiFactory import BelyApiFactory token = load_token() - if not token: - return False - - factory = BelyApiFactory(bely_url=get_host()) - factory.api_client.set_default_header(BelyApiFactory.HEADER_TOKEN_KEY, token) + if factory is None: + if not token: + return False + factory = BelyApiFactory(bely_url=get_host()) + factory.api_client.set_default_header(BelyApiFactory.HEADER_TOKEN_KEY, token) try: factory.logout_user() except belyApi.exceptions.UnauthorizedException: diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/app.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/app.py index 809011b76..57cdd279a 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/app.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/app.py @@ -138,8 +138,12 @@ def get_system_commands(self, screen): "My documents", "Browse your recently modified documents", self._cmd_recent) yield SystemCommand( "Refresh cache", "Discard all cached logbook data", self._cmd_refresh) - yield SystemCommand( - "Log in", "Authenticate now instead of at the first mutation", self._cmd_login) + if self.session.is_authenticated(): + yield SystemCommand( + "Log out", "End the current authenticated session", self._cmd_logout) + else: + yield SystemCommand( + "Log in", "Authenticate now instead of at the first mutation", self._cmd_login) def _cmd_config(self): from .screens.configscreen import ConfigScreen @@ -159,6 +163,20 @@ def _cmd_refresh(self): def _cmd_login(self): self.run_worker(self._do_login(), exclusive=True, group="login") + def _cmd_logout(self): + self.run_worker(self._do_logout(), exclusive=True, group="login") + + async def _do_logout(self): + try: + await asyncio.to_thread(self.session.logout) + except RuntimeError as e: + self.notify(str(e), severity="error") + screen = self.screen + if isinstance(screen, BrowseScreen): + screen._update_auth_status() + if not self.session.is_authenticated(): + self.notify("Logged out.") + async def _do_login(self): api = await self.ensure_auth() screen = self.screen diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/session.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/session.py index df3060a48..89b5ea208 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/session.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/session.py @@ -50,6 +50,14 @@ def login(self, username, password): """ self._auth_factory = auth.login(username, password) + def logout(self): + """Invalidate the active session and remove its cached token.""" + factory = self._auth_factory + try: + return auth.logout(factory) + finally: + self._auth_factory = None + def authenticated_factory(self): """The authenticated BelyApiFactory. Raises RuntimeError if not authenticated yet.""" if self._auth_factory is None: diff --git a/tools/developer_tools/bely-cli/test/test_tui_app.py b/tools/developer_tools/bely-cli/test/test_tui_app.py index 79c8be690..7bde24792 100644 --- a/tools/developer_tools/bely-cli/test/test_tui_app.py +++ b/tools/developer_tools/bely-cli/test/test_tui_app.py @@ -35,6 +35,10 @@ def authenticated_api(self): def try_token(self): return self._authenticated + def logout(self): + self._authenticated = False + return True + class FakeLogbookApi: def get_logbook_type_hierarchy(self): @@ -385,7 +389,7 @@ async def test_q_quits_the_app(self): await pilot.pause() self.assertIsNone(app.return_value) - async def test_command_palette_offers_config_recent_and_login(self): + async def test_command_palette_offers_config_recent_and_logout_when_authenticated(self): data = LogbookData(FakeLogbookApi()) app = BelyTuiApp(FakeSession(data), limit=10, mode="app") async with app.run_test() as pilot: @@ -394,7 +398,33 @@ async def test_command_palette_offers_config_recent_and_login(self): self.assertIn("Configuration", titles) self.assertIn("My documents", titles) self.assertIn("Refresh cache", titles) + self.assertIn("Log out", titles) + self.assertNotIn("Log in", titles) + + async def test_command_palette_offers_login_when_unauthenticated(self): + data = LogbookData(FakeLogbookApi()) + app = BelyTuiApp(FakeSession(data, authenticated=False), limit=10, mode="app") + async with app.run_test() as pilot: + await pilot.pause() + titles = {cmd.title for cmd in app.get_system_commands(app.screen)} self.assertIn("Log in", titles) + self.assertNotIn("Log out", titles) + + async def test_logout_command_clears_session_and_updates_palette(self): + data = LogbookData(FakeLogbookApi()) + session = FakeSession(data) + app = BelyTuiApp(session, limit=10, mode="app") + async with app.run_test() as pilot: + await pilot.pause() + app._cmd_logout() + await pilot.pause() + await pilot.pause() + + self.assertFalse(session.is_authenticated()) + titles = {cmd.title for cmd in app.get_system_commands(app.screen)} + self.assertIn("Log in", titles) + self.assertNotIn("Log out", titles) + self.assertEqual(len(app._notifications), 1) async def test_search_and_resize_bindings_are_gone(self): keys = {b.key for b in BrowseScreen.BINDINGS}