From 83764a9d03b0eeaf8072a2ccdc273e9c36e96ff9 Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Thu, 12 Dec 2024 11:56:08 -0500 Subject: [PATCH] fix: pr suggestion about unifying httpx auth tokens --- craft_store/__init__.py | 3 +- craft_store/{candidauth.py => _httpx_auth.py} | 46 ++++++++---- craft_store/auth.py | 2 +- craft_store/developer_token_auth.py | 75 ------------------- craft_store/publishergateway/_publishergw.py | 8 +- tests/unit/test_candid_auth.py | 45 ----------- ...t_developer_auth.py => test_httpx_auth.py} | 61 ++++++++++----- 7 files changed, 81 insertions(+), 159 deletions(-) rename craft_store/{candidauth.py => _httpx_auth.py} (65%) delete mode 100644 craft_store/developer_token_auth.py delete mode 100644 tests/unit/test_candid_auth.py rename tests/unit/{test_developer_auth.py => test_httpx_auth.py} (61%) diff --git a/craft_store/__init__.py b/craft_store/__init__.py index 7b777a5..4239e4c 100644 --- a/craft_store/__init__.py +++ b/craft_store/__init__.py @@ -21,10 +21,9 @@ from . import creds, endpoints, errors, models, publishergateway +from ._httpx_auth import CandidAuth, DeveloperTokenAuth from .auth import Auth from .base_client import BaseClient -from .candidauth import CandidAuth -from .developer_token_auth import DeveloperTokenAuth from .http_client import HTTPClient from .store_client import StoreClient from .ubuntu_one_store_client import UbuntuOneStoreClient diff --git a/craft_store/candidauth.py b/craft_store/_httpx_auth.py similarity index 65% rename from craft_store/candidauth.py rename to craft_store/_httpx_auth.py index 76d5c3e..9e63b95 100644 --- a/craft_store/candidauth.py +++ b/craft_store/_httpx_auth.py @@ -14,31 +14,30 @@ # You should have received a copy of the GNU Lesser General Public License # along with this program. If not, see . -"""Client for making requests towards publisher gateway.""" +"""Craft Store Authentication Store.""" +import abc +import logging from collections.abc import Generator -from logging import getLogger from typing import Literal import httpx from craft_store import auth, creds, errors -logger = getLogger(__name__) +logger = logging.getLogger(__name__) -class CandidAuth(httpx.Auth): - """Request authentication using developer token.""" +class _TokenAuth(httpx.Auth, metaclass=abc.ABCMeta): + """Base class for httpx token-based authenticators.""" def __init__( - self, - *, - auth: auth.Auth, - auth_type: Literal["bearer", "macaroon"] = "bearer", + self, *, auth: auth.Auth, auth_type: Literal["bearer", "macaroon"] = "bearer" ) -> None: + super().__init__() + self._token: str | None = None self._auth = auth self._auth_type = auth_type - self._token: str | None = None def auth_flow( self, @@ -46,23 +45,22 @@ def auth_flow( ) -> Generator[httpx.Request, httpx.Response, None]: """Update request to include Authorization header.""" if self._token is None: - logger.debug("Getting candid macaroon from keyring") + logger.debug("Getting token from keyring") self._token = self.get_token_from_keyring() self._update_headers(request) yield request + @abc.abstractmethod def get_token_from_keyring(self) -> str: """Get token stored in the credentials storage.""" - logger.debug("Getting candid from credential storage") - return creds.unmarshal_candid_credentials(self._auth.get_credentials()) def _update_headers(self, request: httpx.Request) -> None: """Add token to the request.""" logger.debug("Adding ephemeral token to request headers") if self._token is None: raise errors.DeveloperTokenUnavailableError( - message="Candid token is not available" + message="Token is not available" ) request.headers["Authorization"] = self._format_auth_header() @@ -70,3 +68,23 @@ def _format_auth_header(self) -> str: if self._auth_type == "bearer": return f"Bearer {self._token}" return f"Macaroon {self._token}" + + +class CandidAuth(_TokenAuth): + """Candid based authentication class for httpx store clients.""" + + def get_token_from_keyring(self) -> str: + """Get token stored in the credentials storage.""" + logger.debug("Getting candid from credential storage") + return creds.unmarshal_candid_credentials(self._auth.get_credentials()) + + +class DeveloperTokenAuth(_TokenAuth): + """Developer token based authentication class for httpx store clients.""" + + def get_token_from_keyring(self) -> str: + """Get token stored in the credentials storage.""" + logger.debug("Getting developer token from credential storage") + return creds.DeveloperToken.model_validate_json( + self._auth.get_credentials() + ).macaroon diff --git a/craft_store/auth.py b/craft_store/auth.py index 62400c8..03594a2 100644 --- a/craft_store/auth.py +++ b/craft_store/auth.py @@ -30,7 +30,7 @@ import keyring.backends.fail import keyring.errors from jaraco.classes import properties -from xdg import BaseDirectory # type: ignore[import] +from xdg import BaseDirectory # type: ignore[import-untyped] from . import errors diff --git a/craft_store/developer_token_auth.py b/craft_store/developer_token_auth.py deleted file mode 100644 index f4e8403..0000000 --- a/craft_store/developer_token_auth.py +++ /dev/null @@ -1,75 +0,0 @@ -# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- -# -# Copyright 2024 Canonical Ltd. -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU Lesser General Public -# License version 3 as published by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -# Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public License -# along with this program. If not, see . - -"""Client for making requests towards publisher gateway.""" - -from collections.abc import Generator -from logging import getLogger -from typing import Literal - -import httpx - -from craft_store import auth, creds, errors - -logger = getLogger(__name__) - - -class DeveloperTokenAuth(httpx.Auth): - """Request authentication using developer token.""" - - def __init__( - self, - *, - auth: auth.Auth, - auth_type: Literal["bearer", "macaroon"] = "bearer", - ) -> None: - self._auth = auth - self._auth_type = auth_type - self._token: str | None = None - - def auth_flow( - self, - request: httpx.Request, - ) -> Generator[httpx.Request, httpx.Response, None]: - """Update request to include Authorization header.""" - logger.debug("Adding developer token to authorize request") - if self._token is None: - self.get_token_from_keyring() - - self._update_headers(request) - yield request - - def get_token_from_keyring(self) -> None: - """Get token stored in the credentials storage.""" - logger.debug("Getting developer token from credential storage") - dev_token = creds.DeveloperToken.model_validate_json( - self._auth.get_credentials() - ) - self._token = dev_token.macaroon - - def _update_headers(self, request: httpx.Request) -> None: - """Add token to the request.""" - logger.debug("Adding ephemeral token to request headers") - if self._token is None: - raise errors.DeveloperTokenUnavailableError( - message="Developer token is not available" - ) - request.headers["Authorization"] = self._format_auth_header() - - def _format_auth_header(self) -> str: - if self._auth_type == "bearer": - return f"Bearer {self._token}" - return f"Macaroon {self._token}" diff --git a/craft_store/publishergateway/_publishergw.py b/craft_store/publishergateway/_publishergw.py index 3b3207d..c401302 100644 --- a/craft_store/publishergateway/_publishergw.py +++ b/craft_store/publishergateway/_publishergw.py @@ -19,7 +19,9 @@ import httpx -from craft_store import auth, candidauth, errors +from craft_store import errors +from craft_store._httpx_auth import CandidAuth +from craft_store.auth import Auth from . import _request, _response @@ -33,11 +35,11 @@ class PublisherGateway: Each instance is only valid for one particular namespace. """ - def __init__(self, base_url: str, namespace: str, auth: auth.Auth) -> None: + def __init__(self, base_url: str, namespace: str, auth: Auth) -> None: self._namespace = namespace self._client = httpx.Client( base_url=base_url, - auth=candidauth.CandidAuth(auth=auth, auth_type="macaroon"), + auth=CandidAuth(auth=auth, auth_type="macaroon"), ) @staticmethod diff --git a/tests/unit/test_candid_auth.py b/tests/unit/test_candid_auth.py deleted file mode 100644 index e499479..0000000 --- a/tests/unit/test_candid_auth.py +++ /dev/null @@ -1,45 +0,0 @@ -# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -* -# -# Copyright 2024 Canonical Ltd. -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU Lesser General Public -# License version 3 as published by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -# Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public License -# along with this program. If not, see . -# -"""Tests for authorizing requests using CandidAuth.""" - - -import httpx -import pytest -from craft_store import CandidAuth - - -@pytest.fixture -def candid_auth(mock_auth): - return CandidAuth( - auth=mock_auth, - ) - - -def test_get_token_from_keyring(mock_auth, candid_auth): - mock_auth.get_credentials.return_value = "{}" - - assert candid_auth.get_token_from_keyring() == "{}" - - -def test_auth_flow(mock_auth, candid_auth): - mock_auth.get_credentials.return_value = "{}" - - request = httpx.Request("GET", "http://localhost") - - next(candid_auth.auth_flow(request)) - - assert request.headers["Authorization"] == "Bearer {}" diff --git a/tests/unit/test_developer_auth.py b/tests/unit/test_httpx_auth.py similarity index 61% rename from tests/unit/test_developer_auth.py rename to tests/unit/test_httpx_auth.py index 779b9a6..4bad55e 100644 --- a/tests/unit/test_developer_auth.py +++ b/tests/unit/test_httpx_auth.py @@ -1,20 +1,18 @@ -# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -* +# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- # -# Copyright 2024 Canonical Ltd. +# Copyright 2021,2024 Canonical Ltd. # -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU Lesser General Public -# License version 3 as published by the Free Software Foundation. +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License version 3 as published by the Free Software Foundation. # -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -# Lesser General Public License for more details. +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. # -# You should have received a copy of the GNU Lesser General Public License -# along with this program. If not, see . -# -"""Tests for authorizing requests using DeveloperTokenAuth.""" +# You should have received a copy of the GNU Lesser General Public License +# along with this program. If not, see . from typing import Literal, cast @@ -22,8 +20,31 @@ import pytest import pytest_httpx import pytest_mock -from craft_store import Auth, DeveloperTokenAuth, creds -from craft_store.errors import CredentialsUnavailable, DeveloperTokenUnavailableError +from craft_store import CandidAuth, DeveloperTokenAuth, creds, errors +from craft_store.auth import Auth + + +@pytest.fixture +def candid_auth(mock_auth): + return CandidAuth( + auth=mock_auth, + ) + + +def test_candid_get_token_from_keyring(mock_auth, candid_auth): + mock_auth.get_credentials.return_value = "{}" + + assert candid_auth.get_token_from_keyring() == "{}" + + +def test_candid_auth_flow(mock_auth, candid_auth): + mock_auth.get_credentials.return_value = "{}" + + request = httpx.Request("GET", "http://localhost") + + next(candid_auth.auth_flow(request)) + + assert request.headers["Authorization"] == "Bearer {}" @pytest.fixture @@ -87,7 +108,7 @@ def test_auth_if_token_unavailable() -> None: httpx_client = httpx.Client(auth=developer_token_auth) with pytest.raises( - CredentialsUnavailable, + errors.CredentialsUnavailable, match=f"No credentials found for {app_name!r} on {host!r}.", ): httpx_client.request("GET", "https://fake-testcraft-url.localhost") @@ -98,11 +119,13 @@ def test_auth_if_token_unset( mocker: pytest_mock.MockerFixture, ) -> None: # do not set token that is available in keyring - mocker.patch.object(developer_token_auth, "get_token_from_keyring") + mocker.patch.object( + developer_token_auth, "get_token_from_keyring", return_value=None + ) httpx_client = httpx.Client(auth=developer_token_auth) with pytest.raises( - DeveloperTokenUnavailableError, - match="Developer token is not available", + errors.DeveloperTokenUnavailableError, + match="Token is not available", ): httpx_client.request("GET", "https://fake-testcraft-url.localhost")