Skip to content

Commit

Permalink
Fix missing authentication headers
Browse files Browse the repository at this point in the history
When I refactored the way the HTTP client was used in RemoteButler, I failed to update a few of the locations where it was used.  As a result, some methods were not including the required authentication headers.
  • Loading branch information
dhirving committed Jan 19, 2024
1 parent 4c9f46b commit 0fbf484
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 5 deletions.
10 changes: 5 additions & 5 deletions python/lsst/daf/butler/remote_butler/_remote_butler.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ def dimensions(self) -> DimensionUniverse:
if cache.dimensions is not None:
return cache.dimensions

response = self._client.get(self._get_url("universe"))
response = self._get("universe")
response.raise_for_status()

config = DimensionConfig.fromString(response.text, format="json")
Expand Down Expand Up @@ -334,7 +334,7 @@ def get_dataset_type(self, name: str) -> DatasetType:
# In future implementation this should directly access the cache
# and only go to the server if the dataset type is not known.
path = f"dataset_type/{name}"
response = self._client.get(self._get_url(path))
response = self._get(path)
if response.status_code != httpx.codes.OK:
content = response.json()
if content["exception"] == "MissingDatasetTypeError":
Expand All @@ -357,7 +357,7 @@ def get_dataset(
}
if datastore_records:
raise ValueError("Datastore records can not yet be returned in client/server butler.")
response = self._client.get(self._get_url(path), params=params)
response = self._get(path, params=params)
response.raise_for_status()
if response.json() is None:
return None
Expand Down Expand Up @@ -608,10 +608,10 @@ def _post(self, path: str, model: BaseModel) -> httpx.Response:
url, content=json, headers=self._headers | {"content-type": "application/json"}
)

def _get(self, path: str) -> httpx.Response:
def _get(self, path: str, params: Mapping[str, str | bool] | None = None) -> httpx.Response:
"""Send a GET request to the Butler server."""
url = self._get_url(path)
return self._client.get(url, headers=self._headers)
return self._client.get(url, headers=self._headers, params=params)

def _parse_model(self, response: httpx.Response, model: Type[_AnyPydanticModel]) -> _AnyPydanticModel:
"""Deserialize a Pydantic model from the body of an HTTP response."""
Expand Down
74 changes: 74 additions & 0 deletions python/lsst/daf/butler/tests/server_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# This file is part of daf_butler.
#
# Developed for the LSST Data Management System.
# This product includes software developed by the LSST Project
# (http://www.lsst.org).
# See the COPYRIGHT file at the top-level directory of this distribution
# for details of code ownership.
#
# This software is dual licensed under the GNU General Public License and also
# under a 3-clause BSD license. Recipients may choose which of these licenses
# to use; please see the files gpl-3.0.txt and/or bsd_license.txt,
# respectively. If you choose the GPL option then the following text applies
# (but note that there is still no warranty even if you opt for BSD instead):
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# 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 General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.


import re
from typing import Any

from fastapi import FastAPI, HTTPException, Request, Response


def add_auth_header_check_middleware(app: FastAPI) -> None:
"""Add a middleware to a FastAPI app to check that Gafaelfawr
authentication headers are present.
This is only suitable for testing -- in a real deployment,
GafaelfawrIngress will handle these headers and convert them to a different
format.
Parameters
----------
app : `FastAPI`
The app the middleware will be added to.
"""

@app.middleware("http")
async def check_auth_headers(request: Request, call_next: Any) -> Response:
if _is_authenticated_endpoint(request.url.path):
header = request.headers.get("authorization")
if header is None:
raise HTTPException(status_code=401, detail="Authorization header is missing")
if not re.match(r"^Bearer \S+", header):
raise HTTPException(

Check warning on line 56 in python/lsst/daf/butler/tests/server_utils.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/tests/server_utils.py#L56

Added line #L56 was not covered by tests
status_code=401, detail=f"Authorization header not in expected format: {header}"
)

return await call_next(request)


def _is_authenticated_endpoint(path: str) -> bool:
"""Return True if the specified path requires authentication in the real
server deployment.
"""
if path == "/":
return False
if path.endswith("/butler.yaml"):
return False
if path.endswith("/butler.json"):
return False

Check warning on line 72 in python/lsst/daf/butler/tests/server_utils.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/tests/server_utils.py#L72

Added line #L72 was not covered by tests

return True
12 changes: 12 additions & 0 deletions tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,13 @@

try:
# Failing to import any of these should disable the tests.
from fastapi import HTTPException
from fastapi.testclient import TestClient
from lsst.daf.butler.remote_butler import RemoteButler, RemoteButlerFactory
from lsst.daf.butler.remote_butler._authentication import _EXPLICIT_BUTLER_ACCESS_TOKEN_ENVIRONMENT_KEY
from lsst.daf.butler.remote_butler.server import create_app
from lsst.daf.butler.remote_butler.server._dependencies import butler_factory_dependency
from lsst.daf.butler.tests.server_utils import add_auth_header_check_middleware
from lsst.resources.s3utils import clean_test_environment_for_s3, getS3Client
from moto import mock_s3
except ImportError:
Expand Down Expand Up @@ -121,6 +123,7 @@ def setUpClass(cls):

app = create_app()
app.dependency_overrides[butler_factory_dependency] = lambda: server_butler_factory
add_auth_header_check_middleware(app)

# Set up the RemoteButler that will connect to the server
cls.client = _make_test_client(app)
Expand Down Expand Up @@ -384,6 +387,15 @@ def check_uris(uris: DatasetRefURIs):
componentUris = self.butler.getURIs(componentRef)
check_uris(componentUris)

def test_auth_check(self):
# This is checking that the unit-test middleware for validating the
# authentication headers is working. It doesn't test actual server
# functionality -- in a real deployment, the authentication headers are
# handled by GafaelfawrIngress, not our app.
with self.assertRaises(HTTPException) as cm:
self.client.get("/v1/dataset_type/int")
self.assertEqual(cm.exception.status_code, 401)


def _create_corrupted_dataset(repo: MetricTestRepo) -> DatasetRef:
run = "corrupted-run"
Expand Down

0 comments on commit 0fbf484

Please sign in to comment.