From fc4c51593c2d97b42140120f7cbce31991ff60d7 Mon Sep 17 00:00:00 2001 From: "Mathias L. Baumann" Date: Mon, 14 Jul 2025 13:44:12 +0200 Subject: [PATCH 1/4] Support secrets & signing Signed-off-by: Mathias L. Baumann --- RELEASE_NOTES.md | 4 +- pyproject.toml | 2 +- src/frequenz/client/dispatch/__main__.py | 43 ++++++++++++++------- src/frequenz/client/dispatch/_client.py | 26 ++++++------- src/frequenz/client/dispatch/test/client.py | 2 +- 5 files changed, 46 insertions(+), 31 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index a9f508a8..68b02708 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -10,7 +10,9 @@ ## New Features -* `Dispatch.started_at(now: datetime)` was added as alternative to the `started` property for when users want to use the same `now` for multiple calls, ensuring deterministic return values with respect to the same `now`. +* Support secrets for signing and verifying messages. + * Use the new env variable `DISPATCH_API_SECRET` to set the secret key. + * Use the new `sign_secret` parameter in the `DispatchClient` constructor to set the secret key. ## Bug Fixes diff --git a/pyproject.toml b/pyproject.toml index 988f4758..392cf857 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,7 +38,7 @@ requires-python = ">= 3.11, < 4" dependencies = [ "typing-extensions >= 4.13.0, < 5", "frequenz-api-dispatch == 1.0.0-rc2", - "frequenz-client-base >= 0.8.0, < 0.12.0", + "frequenz-client-base >= 0.11.0, < 0.12.0", "frequenz-client-common >= 0.3.2, < 0.4.0", "frequenz-core >= 1.0.2, < 2.0.0", "grpcio >= 1.70.0, < 2", diff --git a/src/frequenz/client/dispatch/__main__.py b/src/frequenz/client/dispatch/__main__.py index 36d2cfd5..866f45fc 100644 --- a/src/frequenz/client/dispatch/__main__.py +++ b/src/frequenz/client/dispatch/__main__.py @@ -171,12 +171,20 @@ def format_line(key: str, value: str, color: str = "cyan") -> str: show_envvar=True, ) @click.option( - "--key", + "--api-key", help="API key for authentication", envvar="DISPATCH_API_KEY", show_envvar=True, required=True, ) +@click.option( + "--sign-secret", + help="API signing secret for authentication", + envvar="DISPATCH_API_SIGN_SECRET", + show_envvar=True, + required=False, + default=None, +) @click.option( "--raw", is_flag=True, @@ -185,29 +193,36 @@ def format_line(key: str, value: str, color: str = "cyan") -> str: default=False, ) @click.pass_context -async def cli(ctx: click.Context, url: str, key: str, raw: bool) -> None: +async def cli( + ctx: click.Context, url: str, api_key: str, sign_secret: str | None, raw: bool +) -> None: """Dispatch Service CLI.""" if ctx.obj is None: ctx.obj = {} click.echo(f"Using API URL: {url}", err=True) + click.echo(f"Using API Key: {api_key}", err=True) + if sign_secret: + click.echo(f"Using API Secret: {sign_secret}", err=True) ctx.obj["client"] = DispatchApiClient( server_url=url, - key=key, + auth_key=api_key, + sign_secret=sign_secret, connect=True, ) ctx.obj["params"] = { "url": url, - "key": key, + "api_key": api_key, + "sign_secret": sign_secret, } ctx.obj["raw"] = raw # Check if a subcommand was given if ctx.invoked_subcommand is None: - await interactive_mode(url, key) + await interactive_mode(url, api_key, sign_secret) @cli.command("list") @@ -533,8 +548,9 @@ async def repl( obj: dict[str, Any], ) -> None: """Start an interactive interface.""" - click.echo(f"Parameters: {obj}") - await interactive_mode(obj["params"]["url"], obj["params"]["key"]) + await interactive_mode( + obj["params"]["url"], obj["params"]["api_key"], obj["params"]["sign_secret"] + ) @cli.command() @@ -574,7 +590,7 @@ async def delete( raise click.ClickException("Some deletions failed.") -async def interactive_mode(url: str, key: str) -> None: +async def interactive_mode(url: str, api_key: str, sign_secret: str | None) -> None: """Interactive mode for the CLI.""" hist_file = os.path.expanduser("~/.dispatch_cli_history.txt") session: PromptSession[str] = PromptSession(history=FileHistory(filename=hist_file)) @@ -614,12 +630,11 @@ async def display_help() -> None: break else: # Split, but keep quoted strings together - params = [ - "--url", - url, - "--key", - key, - ] + click.parser.split_arg_string(user_input) + params = ( + ["--url", url, "--api-key", api_key] + + (["--sign-secret", sign_secret] if sign_secret else []) + + click.parser.split_arg_string(user_input) + ) try: await cli.main(args=params, standalone_mode=False) diff --git a/src/frequenz/client/dispatch/_client.py b/src/frequenz/client/dispatch/_client.py index 18d2db74..eda766a7 100644 --- a/src/frequenz/client/dispatch/_client.py +++ b/src/frequenz/client/dispatch/_client.py @@ -60,7 +60,8 @@ def __init__( self, *, server_url: str, - key: str, + auth_key: str, + sign_secret: str | None = None, connect: bool = True, call_timeout: timedelta = timedelta(seconds=60), stream_timeout: timedelta = timedelta(minutes=5), @@ -69,7 +70,8 @@ def __init__( Args: server_url: The URL of the server to connect to. - key: API key to use for authentication. + auth_key: API key to use for authentication. + sign_secret: Optional secret for signing requests. connect: Whether to connect to the service immediately. call_timeout: Timeout for gRPC calls, default is 60 seconds. stream_timeout: Timeout for gRPC streams, default is 5 minutes. @@ -82,8 +84,9 @@ def __init__( port=DEFAULT_DISPATCH_PORT, ssl=SslOptions(enabled=True), ), + auth_key=auth_key, + sign_secret=sign_secret, ) - self._metadata = (("key", key),) self._streams: dict[ MicrogridId, GrpcStreamBroadcaster[StreamMicrogridDispatchesResponse, DispatchEvent], @@ -138,7 +141,8 @@ async def list( ```python client = DispatchApiClient( - key="key", + auth_key="key", + sign_secret="secret", # Optional so far server_url="grpc://dispatch.url.goes.here.example.com" ) async for page in client.list(microgrid_id=MicrogridId(1)): @@ -199,7 +203,7 @@ def to_interval( response = await cast( Awaitable[ListMicrogridDispatchesResponse], self.stub.ListMicrogridDispatches( - request, metadata=self._metadata, timeout=self._call_timeout_seconds + request, timeout=self._call_timeout_seconds ), ) @@ -256,7 +260,6 @@ def _get_stream( AsyncIterator[StreamMicrogridDispatchesResponse], self.stub.StreamMicrogridDispatches( request, - metadata=self._metadata, timeout=self._stream_timeout_seconds, ), ), @@ -327,7 +330,6 @@ async def create( # pylint: disable=too-many-positional-arguments Awaitable[CreateMicrogridDispatchResponse], self.stub.CreateMicrogridDispatch( request.to_protobuf(), - metadata=self._metadata, timeout=self._call_timeout_seconds, ), ) @@ -419,9 +421,7 @@ async def update( response = await cast( Awaitable[UpdateMicrogridDispatchResponse], - self.stub.UpdateMicrogridDispatch( - msg, metadata=self._metadata, timeout=self._call_timeout_seconds - ), + self.stub.UpdateMicrogridDispatch(msg, timeout=self._call_timeout_seconds), ) return Dispatch.from_protobuf(response.dispatch) @@ -443,9 +443,7 @@ async def get( ) response = await cast( Awaitable[GetMicrogridDispatchResponse], - self.stub.GetMicrogridDispatch( - request, metadata=self._metadata, timeout=self._call_timeout_seconds - ), + self.stub.GetMicrogridDispatch(request, timeout=self._call_timeout_seconds), ) return Dispatch.from_protobuf(response.dispatch) @@ -464,6 +462,6 @@ async def delete( await cast( Awaitable[None], self.stub.DeleteMicrogridDispatch( - request, metadata=self._metadata, timeout=self._call_timeout_seconds + request, timeout=self._call_timeout_seconds ), ) diff --git a/src/frequenz/client/dispatch/test/client.py b/src/frequenz/client/dispatch/test/client.py index 7d739f38..f6e14179 100644 --- a/src/frequenz/client/dispatch/test/client.py +++ b/src/frequenz/client/dispatch/test/client.py @@ -24,7 +24,7 @@ def __init__( self, ) -> None: """Initialize the mock client.""" - super().__init__(server_url="mock", key=ALL_KEY, connect=False) + super().__init__(server_url="mock", auth_key=ALL_KEY, connect=False) self._stuba: FakeService = FakeService() @property From b0e9bca1929d932005e1acfb6152874af8cb1033 Mon Sep 17 00:00:00 2001 From: "Mathias L. Baumann" Date: Tue, 15 Jul 2025 16:45:59 +0200 Subject: [PATCH 2/4] Fix tests after updating base client Signed-off-by: Mathias L. Baumann --- src/frequenz/client/dispatch/test/_service.py | 61 ------------------- src/frequenz/client/dispatch/test/client.py | 6 +- tests/test_cli.py | 4 +- 3 files changed, 5 insertions(+), 66 deletions(-) diff --git a/src/frequenz/client/dispatch/test/_service.py b/src/frequenz/client/dispatch/test/_service.py index 4192b2fc..dc995fa9 100644 --- a/src/frequenz/client/dispatch/test/_service.py +++ b/src/frequenz/client/dispatch/test/_service.py @@ -40,12 +40,6 @@ from .._internal_types import DispatchCreateRequest from ..types import Dispatch, DispatchEvent, DispatchId, Event -ALL_KEY = "all" -"""Key that has access to all resources in the FakeService.""" - -NONE_KEY = "none" -"""Key that has no access to any resources in the FakeService.""" - _logger = logging.getLogger(__name__) @@ -84,63 +78,21 @@ def refresh_last_id_for(self, microgrid_id: MicrogridId) -> None: self._last_id = max(self._last_id, max(dispatch.id for dispatch in dispatches)) - def _check_access(self, metadata: grpc.aio.Metadata) -> None: - """Check if the access key is valid. - - Args: - metadata: The metadata. - - Raises: - grpc.RpcError: If the access key is invalid. - """ - # metadata is a weird tuple of tuples, we don't like it - metadata_dict = dict(metadata) - - if "key" not in metadata_dict: - raise grpc.RpcError( - grpc.StatusCode.UNAUTHENTICATED, - "No access key provided", - ) - - key = metadata_dict["key"] - - if key is None: - raise grpc.RpcError( - grpc.StatusCode.UNAUTHENTICATED, - "No access key provided", - ) - - if key == NONE_KEY: - raise grpc.RpcError( - grpc.StatusCode.PERMISSION_DENIED, - "Permission denied", - ) - - if key != ALL_KEY: - raise grpc.RpcError( - grpc.StatusCode.UNAUTHENTICATED, - "Invalid access key", - ) - # pylint: disable=invalid-name async def ListMicrogridDispatches( self, request: ListMicrogridDispatchesRequest, - metadata: grpc.aio.Metadata, timeout: int = 5, # pylint: disable=unused-argument ) -> ListMicrogridDispatchesResponse: """List microgrid dispatches. Args: request: The request. - metadata: The metadata. timeout: timeout for the request, ignored in this mock. Returns: The dispatch list. """ - self._check_access(metadata) - grid_dispatches = self.dispatches.get(MicrogridId(request.microgrid_id), []) return ListMicrogridDispatchesResponse( @@ -159,14 +111,12 @@ async def ListMicrogridDispatches( async def StreamMicrogridDispatches( self, request: StreamMicrogridDispatchesRequest, - metadata: grpc.aio.Metadata, timeout: int = 5, # pylint: disable=unused-argument ) -> AsyncIterator[StreamMicrogridDispatchesResponse]: """Stream microgrid dispatches changes. Args: request: The request. - metadata: The metadata. timeout: timeout for the request, ignored in this mock. Returns: @@ -175,8 +125,6 @@ async def StreamMicrogridDispatches( Yields: An event for each dispatch change. """ - self._check_access(metadata) - receiver = self._stream_channel.new_receiver() async for message in receiver: @@ -231,11 +179,9 @@ def _filter_dispatch( async def CreateMicrogridDispatch( self, request: PBDispatchCreateRequest, - metadata: grpc.aio.Metadata, timeout: int = 5, # pylint: disable=unused-argument ) -> CreateMicrogridDispatchResponse: """Create a new dispatch.""" - self._check_access(metadata) microgrid_id = MicrogridId(request.microgrid_id) self._last_id = DispatchId(int(self._last_id) + 1) @@ -261,12 +207,9 @@ async def CreateMicrogridDispatch( async def UpdateMicrogridDispatch( self, request: UpdateMicrogridDispatchRequest, - metadata: grpc.aio.Metadata, timeout: int = 5, # pylint: disable=unused-argument ) -> UpdateMicrogridDispatchResponse: """Update a dispatch.""" - self._check_access(metadata) - microgrid_id = MicrogridId(request.microgrid_id) grid_dispatches = self.dispatches.get(microgrid_id, []) index = next( @@ -355,11 +298,9 @@ async def UpdateMicrogridDispatch( async def GetMicrogridDispatch( self, request: GetMicrogridDispatchRequest, - metadata: grpc.aio.Metadata, timeout: int = 5, # pylint: disable=unused-argument ) -> GetMicrogridDispatchResponse: """Get a single dispatch.""" - self._check_access(metadata) microgrid_id = MicrogridId(request.microgrid_id) grid_dispatches = self.dispatches.get(microgrid_id, []) dispatch = next( @@ -380,11 +321,9 @@ async def GetMicrogridDispatch( async def DeleteMicrogridDispatch( self, request: DeleteMicrogridDispatchRequest, - metadata: grpc.aio.Metadata, timeout: int = 5, # pylint: disable=unused-argument ) -> Empty: """Delete a given dispatch.""" - self._check_access(metadata) microgrid_id = MicrogridId(request.microgrid_id) grid_dispatches = self.dispatches.get(microgrid_id, []) diff --git a/src/frequenz/client/dispatch/test/client.py b/src/frequenz/client/dispatch/test/client.py index f6e14179..4dda0afc 100644 --- a/src/frequenz/client/dispatch/test/client.py +++ b/src/frequenz/client/dispatch/test/client.py @@ -9,9 +9,9 @@ from .. import DispatchApiClient from ..types import Dispatch -from ._service import ALL_KEY, NONE_KEY, FakeService +from ._service import FakeService -__all__ = ["FakeClient", "to_create_params", "ALL_KEY", "NONE_KEY"] +__all__ = ["FakeClient", "to_create_params"] class FakeClient(DispatchApiClient): @@ -24,7 +24,7 @@ def __init__( self, ) -> None: """Initialize the mock client.""" - super().__init__(server_url="mock", auth_key=ALL_KEY, connect=False) + super().__init__(server_url="mock", auth_key="what", connect=False) self._stuba: FakeService = FakeService() @property diff --git a/tests/test_cli.py b/tests/test_cli.py index 627916b3..6e124f81 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -21,7 +21,7 @@ RecurrenceRule, Weekday, ) -from frequenz.client.dispatch.test.client import ALL_KEY, FakeClient +from frequenz.client.dispatch.test.client import FakeClient from frequenz.client.dispatch.types import ( Dispatch, DispatchId, @@ -33,7 +33,7 @@ TEST_NOW = datetime(2023, 1, 1, 0, 0, 0, tzinfo=timezone.utc) """Arbitrary time used as NOW for testing.""" -ENVIRONMENT_VARIABLES = {"DISPATCH_API_KEY": ALL_KEY} +ENVIRONMENT_VARIABLES = {"DISPATCH_API_KEY": "test_key"} @pytest.fixture From ddb594b568b0eb9ab32077bf7e2735bf7a1fdf92 Mon Sep 17 00:00:00 2001 From: "Mathias L. Baumann" Date: Tue, 22 Jul 2025 10:31:00 +0200 Subject: [PATCH 3/4] Mask both Auth Key and Signing Secret Signed-off-by: Mathias L. Baumann --- src/frequenz/client/dispatch/__main__.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/frequenz/client/dispatch/__main__.py b/src/frequenz/client/dispatch/__main__.py index 866f45fc..65ee3c90 100644 --- a/src/frequenz/client/dispatch/__main__.py +++ b/src/frequenz/client/dispatch/__main__.py @@ -201,9 +201,15 @@ async def cli( ctx.obj = {} click.echo(f"Using API URL: {url}", err=True) - click.echo(f"Using API Key: {api_key}", err=True) + click.echo(f"Using API Auth Key: {api_key[:4]}{'*' * 8}", err=True) + if sign_secret: - click.echo(f"Using API Secret: {sign_secret}", err=True) + if len(sign_secret) > 8: + click.echo( + f"Using API Signing Secret: {sign_secret[:4]}{'*' * 8}", err=True + ) + else: + click.echo("Using API Signing Secret (not shown).", err=True) ctx.obj["client"] = DispatchApiClient( server_url=url, From 833e53138e8a1ac51d7afcfac47da74410104f5a Mon Sep 17 00:00:00 2001 From: "Mathias L. Baumann" Date: Tue, 22 Jul 2025 15:38:02 +0200 Subject: [PATCH 4/4] Rename api-key to auth-key and deprecate old parameter Signed-off-by: Mathias L. Baumann --- RELEASE_NOTES.md | 4 +- src/frequenz/client/dispatch/__main__.py | 54 +++++++++++++++++++----- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 68b02708..38979e06 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -11,8 +11,10 @@ ## New Features * Support secrets for signing and verifying messages. - * Use the new env variable `DISPATCH_API_SECRET` to set the secret key. + * Use the new env variable `DISPATCH_API_SIGN_SECRET` to set the secret key. * Use the new `sign_secret` parameter in the `DispatchClient` constructor to set the secret key. +* Added `auth_key` parameter to the `dispatch-cli` and thew env variable `DISPATCH_API_AUTH_KEY` to set the authentication key for the Dispatch API. + ## Bug Fixes diff --git a/src/frequenz/client/dispatch/__main__.py b/src/frequenz/client/dispatch/__main__.py index 65ee3c90..1306e325 100644 --- a/src/frequenz/client/dispatch/__main__.py +++ b/src/frequenz/client/dispatch/__main__.py @@ -172,10 +172,17 @@ def format_line(key: str, value: str, color: str = "cyan") -> str: ) @click.option( "--api-key", - help="API key for authentication", + help="API key for authentication (deprecated, use --auth-key or DISPATCH_API_AUTH_KEY)", envvar="DISPATCH_API_KEY", show_envvar=True, - required=True, + required=False, +) +@click.option( + "--auth-key", + help="API auth key for authentication", + envvar="DISPATCH_API_AUTH_KEY", + show_envvar=True, + required=False, ) @click.option( "--sign-secret", @@ -193,15 +200,28 @@ def format_line(key: str, value: str, color: str = "cyan") -> str: default=False, ) @click.pass_context -async def cli( - ctx: click.Context, url: str, api_key: str, sign_secret: str | None, raw: bool +async def cli( # pylint: disable=too-many-arguments, too-many-positional-arguments + ctx: click.Context, + url: str, + api_key: str | None, + auth_key: str | None, + sign_secret: str | None, + raw: bool, ) -> None: """Dispatch Service CLI.""" if ctx.obj is None: ctx.obj = {} + key = auth_key or api_key + + if not key: + raise click.BadParameter( + "You must provide an API auth key using --auth-key or " + "the DISPATCH_API_AUTH_KEY environment variable." + ) + click.echo(f"Using API URL: {url}", err=True) - click.echo(f"Using API Auth Key: {api_key[:4]}{'*' * 8}", err=True) + click.echo(f"Using API Auth Key: {key[:4]}{'*' * 8}", err=True) if sign_secret: if len(sign_secret) > 8: @@ -211,16 +231,28 @@ async def cli( else: click.echo("Using API Signing Secret (not shown).", err=True) + if api_key and auth_key is None: + click.echo( + click.style( + "Deprecation Notice: The --api-key option and the DISPATCH_API_KEY environment " + "variable are deprecated. " + "Please use --auth-key or set the DISPATCH_API_AUTH_KEY environment variable.", + fg="red", + bold=True, + ), + err=True, + ) + ctx.obj["client"] = DispatchApiClient( server_url=url, - auth_key=api_key, + auth_key=key, sign_secret=sign_secret, connect=True, ) ctx.obj["params"] = { "url": url, - "api_key": api_key, + "auth_key": key, "sign_secret": sign_secret, } @@ -228,7 +260,7 @@ async def cli( # Check if a subcommand was given if ctx.invoked_subcommand is None: - await interactive_mode(url, api_key, sign_secret) + await interactive_mode(url, key, sign_secret) @cli.command("list") @@ -555,7 +587,7 @@ async def repl( ) -> None: """Start an interactive interface.""" await interactive_mode( - obj["params"]["url"], obj["params"]["api_key"], obj["params"]["sign_secret"] + obj["params"]["url"], obj["params"]["auth_key"], obj["params"]["sign_secret"] ) @@ -596,7 +628,7 @@ async def delete( raise click.ClickException("Some deletions failed.") -async def interactive_mode(url: str, api_key: str, sign_secret: str | None) -> None: +async def interactive_mode(url: str, auth_key: str, sign_secret: str | None) -> None: """Interactive mode for the CLI.""" hist_file = os.path.expanduser("~/.dispatch_cli_history.txt") session: PromptSession[str] = PromptSession(history=FileHistory(filename=hist_file)) @@ -637,7 +669,7 @@ async def display_help() -> None: else: # Split, but keep quoted strings together params = ( - ["--url", url, "--api-key", api_key] + ["--url", url, "--auth-key", auth_key] + (["--sign-secret", sign_secret] if sign_secret else []) + click.parser.split_arg_string(user_input) )