From a0346e1c4622d99abee7d63e81eaab5b13aa6168 Mon Sep 17 00:00:00 2001 From: Abubakarr Kamara <72503671+Abubakarr99@users.noreply.github.com> Date: Fri, 4 Aug 2023 16:25:24 +0000 Subject: [PATCH 01/16] PTFE-693: api authentication --- runner_manager/main.py | 42 +++++++++++++++++++++++++++++-- runner_manager/models/settings.py | 3 ++- tests/api/auth_tests.py | 19 ++++++++++++++ 3 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 tests/api/auth_tests.py diff --git a/runner_manager/main.py b/runner_manager/main.py index 6216fdcc..89839d36 100644 --- a/runner_manager/main.py +++ b/runner_manager/main.py @@ -1,13 +1,39 @@ import logging - +from fastapi import Depends, FastAPI, HTTPException, Response, Security, status +from fastapi.security import APIKeyHeader, APIKeyQuery from fastapi import FastAPI, Response -from runner_manager.dependencies import get_queue +from runner_manager.dependencies import get_queue, get_settings from runner_manager.jobs.startup import startup log = logging.getLogger(__name__) app = FastAPI() +api_key_query = APIKeyQuery(name="api-key", auto_error=False) +api_key_header = APIKeyHeader(name="x-api-key", auto_error=False) + +def get_api_key( + api_key_query: str = Security(api_key_query), + api_key_header: str = Security(api_key_header), +) -> str: + settings = get_settings() + if not settings.api_key: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="API Key not configured in settings", + ) + api_key = api_key_query or api_key_header + if not api_key: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="API Key required for this endpoint", + ) + if api_key != settings.api_key.get_secret_value(): + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Invalid API Key", + ) + return api_key @app.on_event("startup") @@ -21,3 +47,15 @@ def startup_event(): @app.get("/_health") def health(): return Response(status_code=200) + + +@app.get("/public") +def public(): + """A public endpoint that does not require any authentication.""" + return "Public Endpoint" + + +@app.get("/private") +def private(api_key: str = Depends(get_api_key)): + """A private endpoint that requires a valid API key to be provided.""" + return f"Private Endpoint. API Key: {api_key}" diff --git a/runner_manager/models/settings.py b/runner_manager/models/settings.py index f942a7f5..a5ecc9b2 100644 --- a/runner_manager/models/settings.py +++ b/runner_manager/models/settings.py @@ -2,7 +2,7 @@ from typing import Any, Dict, Optional import yaml -from pydantic import AnyHttpUrl, BaseSettings, RedisDsn +from pydantic import AnyHttpUrl, BaseSettings, RedisDsn, SecretStr def yaml_config_settings_source(settings: BaseSettings) -> Dict[str, Any]: @@ -24,6 +24,7 @@ class Settings(BaseSettings): name: str = "runner-manager" redis_om_url: Optional[RedisDsn] = None github_base_url: Optional[AnyHttpUrl] = None + api_key: Optional[SecretStr] = None class Config: config: ConfigFile = ConfigFile() diff --git a/tests/api/auth_tests.py b/tests/api/auth_tests.py new file mode 100644 index 00000000..126ee9ad --- /dev/null +++ b/tests/api/auth_tests.py @@ -0,0 +1,19 @@ +import os + + +def test_public_endpoint(client): + response = client.get("/public") + + assert response.status_code == 200 + + +def test_private_endpoint_without_api_key(client): + response = client.get("/private") + assert response.status_code == 401 + + +def test_private_endpoint_with_valid_api_key(client): + os.environ["API_KEY"] = "secret" + headers = {"x-api-key": "secret"} + response = client.get("/private", headers=headers) + assert response.status_code == 200 From 92b0e2599609a1a313580edf45b247bf096d8411 Mon Sep 17 00:00:00 2001 From: Abubakarr Kamara <72503671+Abubakarr99@users.noreply.github.com> Date: Mon, 7 Aug 2023 11:01:02 +0200 Subject: [PATCH 02/16] Apply suggestions from code review Co-authored-by: Thomas Carmet --- runner_manager/main.py | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/runner_manager/main.py b/runner_manager/main.py index 89839d36..61299c63 100644 --- a/runner_manager/main.py +++ b/runner_manager/main.py @@ -18,22 +18,15 @@ def get_api_key( ) -> str: settings = get_settings() if not settings.api_key: - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail="API Key not configured in settings", - ) - api_key = api_key_query or api_key_header - if not api_key: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="API Key required for this endpoint", - ) - if api_key != settings.api_key.get_secret_value(): - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Invalid API Key", - ) - return api_key + return settings.api_key + if api_key_query in [settings.api_key]: + return api_key_query + if api_key_header in [settings.api_key]: + return api_key_header + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Invalid API Key", + ) @app.on_event("startup") From fae39c9e4f57cd1ba7b5d7a7e540d9e3c0e88e45 Mon Sep 17 00:00:00 2001 From: Abubakarr Kamara <72503671+Abubakarr99@users.noreply.github.com> Date: Mon, 7 Aug 2023 12:37:54 +0000 Subject: [PATCH 03/16] PTFE-693: test api endpoint --- runner_manager/main.py | 8 +++++--- tests/api/{auth_tests.py => tests_auth.py} | 5 ++++- 2 files changed, 9 insertions(+), 4 deletions(-) rename tests/api/{auth_tests.py => tests_auth.py} (77%) diff --git a/runner_manager/main.py b/runner_manager/main.py index 93df3c14..04d90a6c 100644 --- a/runner_manager/main.py +++ b/runner_manager/main.py @@ -1,7 +1,7 @@ import logging + from fastapi import Depends, FastAPI, HTTPException, Response, Security, status -from fastapi.security import APIKeyHeader, APIKeyQuery -from fastapi import FastAPI, Response +from fastapi.security.api_key import APIKey, APIKeyHeader, APIKeyQuery from runner_manager.dependencies import get_queue, get_settings from runner_manager.jobs.startup import startup @@ -13,6 +13,7 @@ api_key_query = APIKeyQuery(name="api-key", auto_error=False) api_key_header = APIKeyHeader(name="x-api-key", auto_error=False) + def get_api_key( api_key_query: str = Security(api_key_query), api_key_header: str = Security(api_key_header), @@ -29,6 +30,7 @@ def get_api_key( detail="Invalid API Key", ) + app.include_router(webhook.router) @@ -52,6 +54,6 @@ def public(): @app.get("/private") -def private(api_key: str = Depends(get_api_key)): +def private(api_key: APIKey = Depends(get_api_key)): """A private endpoint that requires a valid API key to be provided.""" return f"Private Endpoint. API Key: {api_key}" diff --git a/tests/api/auth_tests.py b/tests/api/tests_auth.py similarity index 77% rename from tests/api/auth_tests.py rename to tests/api/tests_auth.py index 126ee9ad..013bbcbe 100644 --- a/tests/api/auth_tests.py +++ b/tests/api/tests_auth.py @@ -1,5 +1,7 @@ import os +from runner_manager.models.settings import Settings + def test_public_endpoint(client): response = client.get("/public") @@ -9,7 +11,8 @@ def test_public_endpoint(client): def test_private_endpoint_without_api_key(client): response = client.get("/private") - assert response.status_code == 401 + # for now we are not using api key + assert response.status_code == 200 def test_private_endpoint_with_valid_api_key(client): From ac2d8352acb7fea3b1cd54887010f61930009f81 Mon Sep 17 00:00:00 2001 From: Abubakarr Kamara <72503671+Abubakarr99@users.noreply.github.com> Date: Mon, 7 Aug 2023 12:41:26 +0000 Subject: [PATCH 04/16] PTFE-693: added api key to test workflow --- .github/workflows/tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index cf6b7247..8b05b079 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -27,7 +27,7 @@ jobs: env: REDIS_OM_URL: redis://localhost:6379/0 GITHUB_BASE_URL: http://localhost:4010 - + API_KEY: ${{ secrets.API_KEY }} steps: - uses: actions/checkout@v3 - name: Boot compose services From ef59c87b8da573ec8e8c3f88824c14c643c55ea4 Mon Sep 17 00:00:00 2001 From: Abubakarr Kamara <72503671+Abubakarr99@users.noreply.github.com> Date: Mon, 7 Aug 2023 14:07:28 +0000 Subject: [PATCH 05/16] PTFE-693: api-key runner manager --- runner_manager/main.py | 8 ++++---- tests/api/tests_auth.py | 2 -- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/runner_manager/main.py b/runner_manager/main.py index 04d90a6c..5e374d84 100644 --- a/runner_manager/main.py +++ b/runner_manager/main.py @@ -1,7 +1,7 @@ import logging -from fastapi import Depends, FastAPI, HTTPException, Response, Security, status -from fastapi.security.api_key import APIKey, APIKeyHeader, APIKeyQuery +from fastapi import FastAPI, HTTPException, Response, Security, status +from fastapi.security import APIKeyHeader, APIKeyQuery from runner_manager.dependencies import get_queue, get_settings from runner_manager.jobs.startup import startup @@ -20,7 +20,7 @@ def get_api_key( ) -> str: settings = get_settings() if not settings.api_key: - return settings.api_key + return "" if api_key_query in [settings.api_key]: return api_key_query if api_key_header in [settings.api_key]: @@ -54,6 +54,6 @@ def public(): @app.get("/private") -def private(api_key: APIKey = Depends(get_api_key)): +def private(api_key: str = Security(get_api_key)): """A private endpoint that requires a valid API key to be provided.""" return f"Private Endpoint. API Key: {api_key}" diff --git a/tests/api/tests_auth.py b/tests/api/tests_auth.py index 013bbcbe..389aa2e8 100644 --- a/tests/api/tests_auth.py +++ b/tests/api/tests_auth.py @@ -1,7 +1,5 @@ import os -from runner_manager.models.settings import Settings - def test_public_endpoint(client): response = client.get("/public") From 1f32c6145096a1a29617e3c8f8ad8aaeb76db74f Mon Sep 17 00:00:00 2001 From: Abubakarr Kamara <72503671+Abubakarr99@users.noreply.github.com> Date: Mon, 7 Aug 2023 18:44:04 +0200 Subject: [PATCH 06/16] Apply suggestions from code review Co-authored-by: Thomas Carmet --- runner_manager/main.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runner_manager/main.py b/runner_manager/main.py index 5e374d84..4c27b705 100644 --- a/runner_manager/main.py +++ b/runner_manager/main.py @@ -1,6 +1,6 @@ import logging -from fastapi import FastAPI, HTTPException, Response, Security, status +from fastapi import FastAPI, HTTPException, Response, Security, status, Depends from fastapi.security import APIKeyHeader, APIKeyQuery from runner_manager.dependencies import get_queue, get_settings @@ -17,8 +17,8 @@ def get_api_key( api_key_query: str = Security(api_key_query), api_key_header: str = Security(api_key_header), + settings: Settings = Depends(get_settings), ) -> str: - settings = get_settings() if not settings.api_key: return "" if api_key_query in [settings.api_key]: From ac9bcd54e0f5f155ca0d5318b9542b990b66d94f Mon Sep 17 00:00:00 2001 From: Abubakarr Kamara <72503671+Abubakarr99@users.noreply.github.com> Date: Tue, 8 Aug 2023 09:22:26 +0200 Subject: [PATCH 07/16] Apply suggestions from code review Co-authored-by: Thomas Carmet --- runner_manager/main.py | 1 + 1 file changed, 1 insertion(+) diff --git a/runner_manager/main.py b/runner_manager/main.py index 4c27b705..e7cae679 100644 --- a/runner_manager/main.py +++ b/runner_manager/main.py @@ -3,6 +3,7 @@ from fastapi import FastAPI, HTTPException, Response, Security, status, Depends from fastapi.security import APIKeyHeader, APIKeyQuery +from runner_manager.models.settings import Settings from runner_manager.dependencies import get_queue, get_settings from runner_manager.jobs.startup import startup from runner_manager.routers import webhook From 553b60c6b3ebdc4a8aada8baf5f5dc86f25859ff Mon Sep 17 00:00:00 2001 From: Abubakarr Kamara <72503671+Abubakarr99@users.noreply.github.com> Date: Tue, 8 Aug 2023 09:20:46 +0000 Subject: [PATCH 08/16] PTFE-693: added setting override --- tests/api/tests_auth.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/api/tests_auth.py b/tests/api/tests_auth.py index 389aa2e8..66fbce29 100644 --- a/tests/api/tests_auth.py +++ b/tests/api/tests_auth.py @@ -1,5 +1,7 @@ import os - +from functools import lru_cache +from runner_manager.models.settings import Settings +from runner_manager.dependencies import get_settings def test_public_endpoint(client): response = client.get("/public") @@ -13,8 +15,14 @@ def test_private_endpoint_without_api_key(client): assert response.status_code == 200 -def test_private_endpoint_with_valid_api_key(client): - os.environ["API_KEY"] = "secret" - headers = {"x-api-key": "secret"} +@lru_cache +def settings_api_key(): + return Settings(api_key="secret") + +def test_private_endpoint_with_valid_api_key(fastapp, client): + settings_api_key.cache_clear() + fastapp.dependency_overrides = {} + fastapp.dependency_overrides[get_settings] = settings_api_key + headers = {"x-api-header": "secret"} response = client.get("/private", headers=headers) assert response.status_code == 200 From b3482f0d9531847971ec02743d08a1c5f6a8a796 Mon Sep 17 00:00:00 2001 From: Abubakarr Kamara <72503671+Abubakarr99@users.noreply.github.com> Date: Tue, 8 Aug 2023 09:21:35 +0000 Subject: [PATCH 09/16] PTFE-693: trunk fmt --- runner_manager/main.py | 4 ++-- tests/api/tests_auth.py | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/runner_manager/main.py b/runner_manager/main.py index e7cae679..4ddd4875 100644 --- a/runner_manager/main.py +++ b/runner_manager/main.py @@ -1,11 +1,11 @@ import logging -from fastapi import FastAPI, HTTPException, Response, Security, status, Depends +from fastapi import Depends, FastAPI, HTTPException, Response, Security, status from fastapi.security import APIKeyHeader, APIKeyQuery -from runner_manager.models.settings import Settings from runner_manager.dependencies import get_queue, get_settings from runner_manager.jobs.startup import startup +from runner_manager.models.settings import Settings from runner_manager.routers import webhook log = logging.getLogger(__name__) diff --git a/tests/api/tests_auth.py b/tests/api/tests_auth.py index 66fbce29..28b57475 100644 --- a/tests/api/tests_auth.py +++ b/tests/api/tests_auth.py @@ -1,7 +1,8 @@ -import os from functools import lru_cache -from runner_manager.models.settings import Settings + from runner_manager.dependencies import get_settings +from runner_manager.models.settings import Settings + def test_public_endpoint(client): response = client.get("/public") @@ -19,6 +20,7 @@ def test_private_endpoint_without_api_key(client): def settings_api_key(): return Settings(api_key="secret") + def test_private_endpoint_with_valid_api_key(fastapp, client): settings_api_key.cache_clear() fastapp.dependency_overrides = {} From 56aa910177c28e84abb08c0947c45a102f67dec6 Mon Sep 17 00:00:00 2001 From: Thomas Carmet Date: Wed, 9 Aug 2023 00:32:51 +0000 Subject: [PATCH 10/16] Identified issue with api key authentication --- runner_manager/main.py | 4 ++-- tests/api/tests_auth.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/runner_manager/main.py b/runner_manager/main.py index 4ddd4875..bae9b695 100644 --- a/runner_manager/main.py +++ b/runner_manager/main.py @@ -22,9 +22,9 @@ def get_api_key( ) -> str: if not settings.api_key: return "" - if api_key_query in [settings.api_key]: + if api_key_query in [settings.api_key.get_secret_value()]: return api_key_query - if api_key_header in [settings.api_key]: + if api_key_header in [settings.api_key.get_secret_value()]: return api_key_header raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, diff --git a/tests/api/tests_auth.py b/tests/api/tests_auth.py index 28b57475..6d11d36c 100644 --- a/tests/api/tests_auth.py +++ b/tests/api/tests_auth.py @@ -25,6 +25,7 @@ def test_private_endpoint_with_valid_api_key(fastapp, client): settings_api_key.cache_clear() fastapp.dependency_overrides = {} fastapp.dependency_overrides[get_settings] = settings_api_key - headers = {"x-api-header": "secret"} + headers = {"x-api-key": "secret"} response = client.get("/private", headers=headers) + print(response.text) assert response.status_code == 200 From c814f6fc6bc67a0fe588482f52820956c71f641c Mon Sep 17 00:00:00 2001 From: Thomas Carmet Date: Wed, 9 Aug 2023 02:46:33 +0000 Subject: [PATCH 11/16] Add middleware for healthcheck --- runner_manager/auth.py | 16 ++++++++++++++++ runner_manager/main.py | 15 ++++++++++----- runner_manager/models/settings.py | 6 +++++- runner_manager/routers/_health.py | 10 ++++++++++ tests/api/tests_auth.py | 1 - 5 files changed, 41 insertions(+), 7 deletions(-) create mode 100644 runner_manager/auth.py create mode 100644 runner_manager/routers/_health.py diff --git a/runner_manager/auth.py b/runner_manager/auth.py new file mode 100644 index 00000000..eceb6622 --- /dev/null +++ b/runner_manager/auth.py @@ -0,0 +1,16 @@ + +from fastapi.middleware.trustedhost import TrustedHostMiddleware +from starlette.types import Receive, Scope, Send + + +class TrustedHostHealthRoutes(TrustedHostMiddleware): + """A healthcheck endpoint that answers to GET requests on /_health""" + + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + """If the request is made on the path _health then execute the check on hosts""" + if scope["path"] == "/_health/": + print(scope) + await super().__call__(scope, receive, send) + else: + await self.app(scope, receive, send) diff --git a/runner_manager/main.py b/runner_manager/main.py index bae9b695..8f9c512b 100644 --- a/runner_manager/main.py +++ b/runner_manager/main.py @@ -1,18 +1,20 @@ import logging -from fastapi import Depends, FastAPI, HTTPException, Response, Security, status +from fastapi import Depends, FastAPI, HTTPException, Security, status from fastapi.security import APIKeyHeader, APIKeyQuery +from runner_manager.auth import TrustedHostHealthRoutes from runner_manager.dependencies import get_queue, get_settings from runner_manager.jobs.startup import startup from runner_manager.models.settings import Settings -from runner_manager.routers import webhook +from runner_manager.routers import webhook, _health log = logging.getLogger(__name__) app = FastAPI() api_key_query = APIKeyQuery(name="api-key", auto_error=False) api_key_header = APIKeyHeader(name="x-api-key", auto_error=False) +settings = get_settings() def get_api_key( @@ -20,6 +22,7 @@ def get_api_key( api_key_header: str = Security(api_key_header), settings: Settings = Depends(get_settings), ) -> str: + """Get the API key from either the query parameter or the header""" if not settings.api_key: return "" if api_key_query in [settings.api_key.get_secret_value()]: @@ -33,6 +36,11 @@ def get_api_key( app.include_router(webhook.router) +app.include_router(_health.router) +app.add_middleware( + TrustedHostHealthRoutes, + allowed_hosts=settings.allowed_hosts +) @app.on_event("startup") @@ -43,9 +51,6 @@ def startup_event(): log.info(f"Startup job is {status}") -@app.get("/_health") -def health(): - return Response(status_code=200) @app.get("/public") diff --git a/runner_manager/models/settings.py b/runner_manager/models/settings.py index 7ad29351..d720d6d8 100644 --- a/runner_manager/models/settings.py +++ b/runner_manager/models/settings.py @@ -1,5 +1,5 @@ from pathlib import Path -from typing import Any, Dict, Optional +from typing import Any, Dict, Optional, Sequence import yaml from pydantic import AnyHttpUrl, BaseSettings, RedisDsn, SecretStr @@ -26,6 +26,10 @@ class Settings(BaseSettings): redis_om_url: Optional[RedisDsn] = None github_base_url: Optional[AnyHttpUrl] = None api_key: Optional[SecretStr] = None + allowed_hosts: Optional[Sequence[str]] = [ + "localhost", + "testserver", + ] class Config: smart_union = True diff --git a/runner_manager/routers/_health.py b/runner_manager/routers/_health.py new file mode 100644 index 00000000..52a50bfb --- /dev/null +++ b/runner_manager/routers/_health.py @@ -0,0 +1,10 @@ + +from fastapi import APIRouter, Response + +router = APIRouter(prefix="/_health") + + +@router.get("/", status_code=200) +def healthcheck(): + """Healthcheck endpoint that answers to GET requests on /_healthz""" + return Response(status_code=200) diff --git a/tests/api/tests_auth.py b/tests/api/tests_auth.py index 6d11d36c..b68d19ee 100644 --- a/tests/api/tests_auth.py +++ b/tests/api/tests_auth.py @@ -27,5 +27,4 @@ def test_private_endpoint_with_valid_api_key(fastapp, client): fastapp.dependency_overrides[get_settings] = settings_api_key headers = {"x-api-key": "secret"} response = client.get("/private", headers=headers) - print(response.text) assert response.status_code == 200 From d35d67bf2101346de2c8d6b615cfec38bc6575b5 Mon Sep 17 00:00:00 2001 From: Abubakarr Kamara <72503671+Abubakarr99@users.noreply.github.com> Date: Wed, 9 Aug 2023 09:23:13 +0200 Subject: [PATCH 12/16] Apply suggestions from code review Co-authored-by: Thomas Carmet --- .github/workflows/tests.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 8b05b079..2626ce77 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -27,7 +27,6 @@ jobs: env: REDIS_OM_URL: redis://localhost:6379/0 GITHUB_BASE_URL: http://localhost:4010 - API_KEY: ${{ secrets.API_KEY }} steps: - uses: actions/checkout@v3 - name: Boot compose services From 5274caf3f1934dcbcc113fc5ac114237fc2cfc4d Mon Sep 17 00:00:00 2001 From: Abubakarr Kamara <72503671+Abubakarr99@users.noreply.github.com> Date: Wed, 9 Aug 2023 07:38:21 +0000 Subject: [PATCH 13/16] PTFE-693: test api authentication --- runner_manager/auth.py | 30 ++++++++++++++++++++++++-- runner_manager/main.py | 36 ++++--------------------------- runner_manager/routers/_health.py | 1 - tests/api/tests_auth.py | 19 +++++++++------- 4 files changed, 43 insertions(+), 43 deletions(-) diff --git a/runner_manager/auth.py b/runner_manager/auth.py index eceb6622..083b0279 100644 --- a/runner_manager/auth.py +++ b/runner_manager/auth.py @@ -1,12 +1,15 @@ - +from fastapi import Depends, HTTPException, Security, status from fastapi.middleware.trustedhost import TrustedHostMiddleware +from fastapi.security import APIKeyHeader, APIKeyQuery from starlette.types import Receive, Scope, Send +from runner_manager.dependencies import get_settings +from runner_manager.models.settings import Settings + class TrustedHostHealthRoutes(TrustedHostMiddleware): """A healthcheck endpoint that answers to GET requests on /_health""" - async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: """If the request is made on the path _health then execute the check on hosts""" if scope["path"] == "/_health/": @@ -14,3 +17,26 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: await super().__call__(scope, receive, send) else: await self.app(scope, receive, send) + + +api_key_query = APIKeyQuery(name="api-key", auto_error=False) +api_key_header = APIKeyHeader(name="x-api-key", auto_error=False) +settings = get_settings() + + +def get_api_key( + api_key_query: str = Security(api_key_query), + api_key_header: str = Security(api_key_header), + settings: Settings = Depends(get_settings), +) -> str: + """Get the API key from either the query parameter or the header""" + if not settings.api_key: + return "" + if api_key_query in [settings.api_key.get_secret_value()]: + return api_key_query + if api_key_header in [settings.api_key.get_secret_value()]: + return api_key_header + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Invalid API Key", + ) diff --git a/runner_manager/main.py b/runner_manager/main.py index 8f9c512b..419126f2 100644 --- a/runner_manager/main.py +++ b/runner_manager/main.py @@ -1,46 +1,20 @@ import logging -from fastapi import Depends, FastAPI, HTTPException, Security, status -from fastapi.security import APIKeyHeader, APIKeyQuery -from runner_manager.auth import TrustedHostHealthRoutes +from fastapi import FastAPI, Security +from runner_manager.auth import TrustedHostHealthRoutes, get_api_key from runner_manager.dependencies import get_queue, get_settings from runner_manager.jobs.startup import startup -from runner_manager.models.settings import Settings -from runner_manager.routers import webhook, _health +from runner_manager.routers import _health, webhook log = logging.getLogger(__name__) - app = FastAPI() -api_key_query = APIKeyQuery(name="api-key", auto_error=False) -api_key_header = APIKeyHeader(name="x-api-key", auto_error=False) settings = get_settings() -def get_api_key( - api_key_query: str = Security(api_key_query), - api_key_header: str = Security(api_key_header), - settings: Settings = Depends(get_settings), -) -> str: - """Get the API key from either the query parameter or the header""" - if not settings.api_key: - return "" - if api_key_query in [settings.api_key.get_secret_value()]: - return api_key_query - if api_key_header in [settings.api_key.get_secret_value()]: - return api_key_header - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Invalid API Key", - ) - - app.include_router(webhook.router) app.include_router(_health.router) -app.add_middleware( - TrustedHostHealthRoutes, - allowed_hosts=settings.allowed_hosts -) +app.add_middleware(TrustedHostHealthRoutes, allowed_hosts=settings.allowed_hosts) @app.on_event("startup") @@ -51,8 +25,6 @@ def startup_event(): log.info(f"Startup job is {status}") - - @app.get("/public") def public(): """A public endpoint that does not require any authentication.""" diff --git a/runner_manager/routers/_health.py b/runner_manager/routers/_health.py index 52a50bfb..4add2c54 100644 --- a/runner_manager/routers/_health.py +++ b/runner_manager/routers/_health.py @@ -1,4 +1,3 @@ - from fastapi import APIRouter, Response router = APIRouter(prefix="/_health") diff --git a/tests/api/tests_auth.py b/tests/api/tests_auth.py index b68d19ee..d472896a 100644 --- a/tests/api/tests_auth.py +++ b/tests/api/tests_auth.py @@ -4,21 +4,24 @@ from runner_manager.models.settings import Settings +@lru_cache +def settings_api_key(): + return Settings(api_key="secret") + + def test_public_endpoint(client): response = client.get("/public") assert response.status_code == 200 -def test_private_endpoint_without_api_key(client): +def test_private_endpoint_without_api_key(fastapp, client): response = client.get("/private") - # for now we are not using api key - assert response.status_code == 200 - - -@lru_cache -def settings_api_key(): - return Settings(api_key="secret") + settings_api_key.cache_clear() + fastapp.dependency_overrides = {} + fastapp.dependency_overrides[get_settings] = settings_api_key + response = client.get("/private") + assert response.status_code == 401 def test_private_endpoint_with_valid_api_key(fastapp, client): From 11017fcbe37a8032401eeac3a7e3fb509a128775 Mon Sep 17 00:00:00 2001 From: Abubakarr Kamara <72503671+Abubakarr99@users.noreply.github.com> Date: Wed, 9 Aug 2023 07:42:58 +0000 Subject: [PATCH 14/16] PTFE-693: renamed auth test file --- tests/api/{tests_auth.py => test_auth.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/api/{tests_auth.py => test_auth.py} (100%) diff --git a/tests/api/tests_auth.py b/tests/api/test_auth.py similarity index 100% rename from tests/api/tests_auth.py rename to tests/api/test_auth.py From f08428b105a036fb384d3ca5e4f5ddc3a3adeac9 Mon Sep 17 00:00:00 2001 From: Abubakarr Kamara <72503671+Abubakarr99@users.noreply.github.com> Date: Thu, 10 Aug 2023 10:29:25 +0200 Subject: [PATCH 15/16] Apply suggestions from code review Co-authored-by: Thomas Carmet --- runner_manager/auth.py | 8 ++++---- runner_manager/routers/_health.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/runner_manager/auth.py b/runner_manager/auth.py index 083b0279..ce1c0a55 100644 --- a/runner_manager/auth.py +++ b/runner_manager/auth.py @@ -7,6 +7,10 @@ from runner_manager.models.settings import Settings +api_key_query = APIKeyQuery(name="api-key", auto_error=False) +api_key_header = APIKeyHeader(name="x-api-key", auto_error=False) + + class TrustedHostHealthRoutes(TrustedHostMiddleware): """A healthcheck endpoint that answers to GET requests on /_health""" @@ -17,10 +21,6 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: await super().__call__(scope, receive, send) else: await self.app(scope, receive, send) - - -api_key_query = APIKeyQuery(name="api-key", auto_error=False) -api_key_header = APIKeyHeader(name="x-api-key", auto_error=False) settings = get_settings() diff --git a/runner_manager/routers/_health.py b/runner_manager/routers/_health.py index 4add2c54..5927db75 100644 --- a/runner_manager/routers/_health.py +++ b/runner_manager/routers/_health.py @@ -5,5 +5,5 @@ @router.get("/", status_code=200) def healthcheck(): - """Healthcheck endpoint that answers to GET requests on /_healthz""" + """Healthcheck endpoint that answers to GET requests on /_health""" return Response(status_code=200) From 599f7f8e5f980e63c46263136cf15b3acf9ceaf2 Mon Sep 17 00:00:00 2001 From: Abubakarr Kamara <72503671+Abubakarr99@users.noreply.github.com> Date: Thu, 10 Aug 2023 09:18:06 +0000 Subject: [PATCH 16/16] PTFE-693: add router for private and public endpoints --- runner_manager/auth.py | 2 -- runner_manager/main.py | 20 +++++--------------- runner_manager/routers/private.py | 11 +++++++++++ runner_manager/routers/public.py | 9 +++++++++ 4 files changed, 25 insertions(+), 17 deletions(-) create mode 100644 runner_manager/routers/private.py create mode 100644 runner_manager/routers/public.py diff --git a/runner_manager/auth.py b/runner_manager/auth.py index ce1c0a55..26e4ed7f 100644 --- a/runner_manager/auth.py +++ b/runner_manager/auth.py @@ -6,7 +6,6 @@ from runner_manager.dependencies import get_settings from runner_manager.models.settings import Settings - api_key_query = APIKeyQuery(name="api-key", auto_error=False) api_key_header = APIKeyHeader(name="x-api-key", auto_error=False) @@ -21,7 +20,6 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: await super().__call__(scope, receive, send) else: await self.app(scope, receive, send) -settings = get_settings() def get_api_key( diff --git a/runner_manager/main.py b/runner_manager/main.py index 419126f2..cd85159b 100644 --- a/runner_manager/main.py +++ b/runner_manager/main.py @@ -1,11 +1,11 @@ import logging -from fastapi import FastAPI, Security +from fastapi import FastAPI -from runner_manager.auth import TrustedHostHealthRoutes, get_api_key +from runner_manager.auth import TrustedHostHealthRoutes from runner_manager.dependencies import get_queue, get_settings from runner_manager.jobs.startup import startup -from runner_manager.routers import _health, webhook +from runner_manager.routers import _health, private, public, webhook log = logging.getLogger(__name__) app = FastAPI() @@ -14,6 +14,8 @@ app.include_router(webhook.router) app.include_router(_health.router) +app.include_router(private.router) +app.include_router(public.router) app.add_middleware(TrustedHostHealthRoutes, allowed_hosts=settings.allowed_hosts) @@ -23,15 +25,3 @@ def startup_event(): job = queue.enqueue(startup) status = job.get_status() log.info(f"Startup job is {status}") - - -@app.get("/public") -def public(): - """A public endpoint that does not require any authentication.""" - return "Public Endpoint" - - -@app.get("/private") -def private(api_key: str = Security(get_api_key)): - """A private endpoint that requires a valid API key to be provided.""" - return f"Private Endpoint. API Key: {api_key}" diff --git a/runner_manager/routers/private.py b/runner_manager/routers/private.py new file mode 100644 index 00000000..4d62c3cf --- /dev/null +++ b/runner_manager/routers/private.py @@ -0,0 +1,11 @@ +from fastapi import APIRouter, Security + +from runner_manager.auth import get_api_key + +router = APIRouter(prefix="/private") + + +@router.get("/") +def private(api_key: str = Security(get_api_key)): + """A private endpoint that requires a valid API key to be provided.""" + return f"Private Endpoint. API Key: {api_key}" diff --git a/runner_manager/routers/public.py b/runner_manager/routers/public.py new file mode 100644 index 00000000..e110d9c7 --- /dev/null +++ b/runner_manager/routers/public.py @@ -0,0 +1,9 @@ +from fastapi import APIRouter + +router = APIRouter(prefix="/public") + + +@router.get("/") +def public(): + """A public endpoint that does not require any authentication.""" + return "Public Endpoint"