From d5992903d194672e82ee0fa93cfad6c24765b4b7 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Thu, 16 Jan 2025 10:46:22 +0000 Subject: [PATCH] Add better error handling for login and docs for authentication --- docs/how-to/authenticate.md | 67 +++++++++++++++++++++++++++ src/blueapi/cli/cli.py | 6 +++ src/blueapi/service/authentication.py | 15 ++++++ tests/unit_tests/test_cli.py | 39 ++++++++++++++-- 4 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 docs/how-to/authenticate.md diff --git a/docs/how-to/authenticate.md b/docs/how-to/authenticate.md new file mode 100644 index 000000000..825b7d9a4 --- /dev/null +++ b/docs/how-to/authenticate.md @@ -0,0 +1,67 @@ +# Authenticate to BlueAPI + +## Introduction +BlueAPI provides a secure and efficient way to interact with its services. This guide walks you through the steps to log in and log out using BlueAPI with OpenID Connect (OIDC) authentication. + +## Configuration + +:::{seealso} +[Configure the Application](./configure-app.md) +::: + +Here is an example configuration for authenticating to p46-blueapi: + +```yaml +api: + host: "p46-blueapi.diamond.ac.uk" + port: 443 + protocol: "https" + +auth_token_path: "~/.cache/blueapi_cache" # Optional: Custom path to store the token +``` + +- **auth_token_path**: (Optional) Specify where to save the token. If omitted, the default is `~/.cache/blueapi_cache` or `$XDG_CACHE_HOME/blueapi_cache` if `XDG_CACHE_HOME` is set. + +--- + +## Log In + +1. Execute the login command: + + ```bash + $ blueapi -c config.yaml login + ``` + +2. **Authenticate**: + - Follow the prompts from your OIDC provider to log in. + - Provide your credentials and complete any additional verification steps required by the provider. + +3. **Success Message**: + Upon successful authentication, you see the following message: + + ``` + Logged in and cached new token + ``` + +--- + +## Log Out + +To log out and securely remove the cached access token, follow these steps: + +1. Execute the logout command: + + ```bash + $ blueapi logout + ``` + +2. **Logout Process**: + - This command uses the OIDC flow to log you out from the OIDC provider. + - It also deletes the cached token from the specified `auth_token_path`. + +3. **Success Message**: + If the token is successfully removed or if it does not exist, you see the message: + + ``` + Logged out + ``` diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 2697ec63d..8c922c5b5 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -363,6 +363,9 @@ def scratch(obj: dict) -> None: @check_connection @click.pass_obj def login(obj: dict) -> None: + """ + Authenticate with the blueapi using the OIDC (OpenID Connect) flow. + """ config: ApplicationConfig = obj["config"] try: auth: SessionManager = SessionManager.from_cache(config.auth_token_path) @@ -381,6 +384,9 @@ def login(obj: dict) -> None: @main.command(name="logout") @click.pass_obj def logout(obj: dict) -> None: + """ + Logs out from the OIDC provider and removes the cached access token. + """ config: ApplicationConfig = obj["config"] try: auth: SessionManager = SessionManager.from_cache(config.auth_token_path) diff --git a/src/blueapi/service/authentication.py b/src/blueapi/service/authentication.py index df3df8a25..edf960c7d 100644 --- a/src/blueapi/service/authentication.py +++ b/src/blueapi/service/authentication.py @@ -23,6 +23,8 @@ class CacheManager(ABC): + @abstractmethod + def can_access_cache(self) -> bool: ... @abstractmethod def save_cache(self, cache: Cache) -> None: ... @abstractmethod @@ -63,6 +65,18 @@ def _default_token_cache_path(self) -> Path: cache_path = os.environ.get("XDG_CACHE_HOME", DEFAULT_CAHCE_DIR) return Path(cache_path).expanduser() / "blueapi_cache" + def can_access_cache(self) -> bool: + assert self._token_path + try: + self._token_path.write_text("") + except IsADirectoryError: + print("Invalid path: a directory path was provided instead of a file path") + return False + except PermissionError: + print(f"Permission denied: Cannot write to {self._token_path.absolute()}") + return False + return True + class SessionManager: def __init__(self, server_config: OIDCConfig, cache_manager: CacheManager) -> None: @@ -179,6 +193,7 @@ def poll_for_token( raise TimeoutError("Polling timed out") def start_device_flow(self): + assert self._cache_manager.can_access_cache() print("Logging in") response: requests.Response = requests.post( self._server_config.device_authorization_endpoint, diff --git a/tests/unit_tests/test_cli.py b/tests/unit_tests/test_cli.py index fb918fb66..93ed0c243 100644 --- a/tests/unit_tests/test_cli.py +++ b/tests/unit_tests/test_cli.py @@ -9,6 +9,7 @@ import pytest import responses +import yaml from bluesky_stomp.messaging import StompClient from click.testing import CliRunner from pydantic import BaseModel, ValidationError @@ -20,7 +21,7 @@ from blueapi.cli.cli import main from blueapi.cli.format import OutputFormat, fmt_dict from blueapi.client.rest import BlueskyRemoteControlError -from blueapi.config import ScratchConfig, ScratchRepository +from blueapi.config import ApplicationConfig, ScratchConfig, ScratchRepository from blueapi.core.bluesky_types import DataEvent, Plan from blueapi.service.model import ( DeviceModel, @@ -329,9 +330,9 @@ def test_env_reload_server_side_error(runner: CliRunner): ) result = runner.invoke(main, ["controller", "env", "-r"]) - assert isinstance( - result.exception, BlueskyRemoteControlError - ), "Expected a BlueskyRemoteError from cli runner" + assert isinstance(result.exception, BlueskyRemoteControlError), ( + "Expected a BlueskyRemoteError from cli runner" + ) assert result.exception.args[0] == "Failed to tear down the environment" # Check if the endpoints were hit as expected @@ -745,3 +746,33 @@ def test_local_cache_cleared_on_logout_when_oidc_unavailable( in result.output ) assert not cached_valid_refresh.exists() + + +def test_wrapper_is_a_directory_error( + runner: CliRunner, mock_authn_server: responses.RequestsMock, tmp_path +): + config: ApplicationConfig = ApplicationConfig(auth_token_path=tmp_path) + config_path = tmp_path / "config.yaml" + with open(config_path, mode="w") as valid_auth_config_file: + valid_auth_config_file.write(yaml.dump(config.model_dump())) + result = runner.invoke(main, ["-c", config_path.as_posix(), "login"]) + assert ( + "Invalid path: a directory path was provided instead of a file path\n" + == result.stdout + ) + + +def test_wrapper_permission_error( + runner: CliRunner, mock_authn_server: responses.RequestsMock, tmp_path +): + token_file: Path = tmp_path / "dir/token" + token_file.parent.mkdir() + # Change the dir permissions to read-only + (tmp_path / "dir").chmod(0o400) + + config: ApplicationConfig = ApplicationConfig(auth_token_path=token_file) + config_path = tmp_path / "config.yaml" + with open(config_path, mode="w") as valid_auth_config_file: + valid_auth_config_file.write(yaml.dump(config.model_dump())) + result = runner.invoke(main, ["-c", config_path.as_posix(), "login"]) + assert f"Permission denied: Cannot write to {token_file}\n" == result.stdout