From 7dc871e23ba003759e5663f8c7de1cbdce43db54 Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Wed, 26 Apr 2023 12:39:36 -0400 Subject: [PATCH] Add API Key functional testing (#3401) PBENCH-1135 Add mechanism and a test case to verify that we can authenticate and identify the client user from an API key, simplistically using `GET /datasets?mine`. This also adds a new `alembic-migration` command, `show` to display what alembic sees as the "heads" and "history" of the revision chain. I added this when I accidentally constructed multiple heads and got weird errors while I was building the `AuthType.API_KEY` upgrade, but didn't include it in that final PR. It seems useful, so I'm sweeping it in here. --- lib/pbench/client/__init__.py | 29 +++++++++++++++++-- lib/pbench/client/types.py | 12 ++++++++ lib/pbench/server/database/alembic.migration | 3 ++ lib/pbench/test/functional/server/test_put.py | 22 ++++++++++++++ 4 files changed, 63 insertions(+), 3 deletions(-) diff --git a/lib/pbench/client/__init__.py b/lib/pbench/client/__init__.py index d9358dcfa0..3eaf0d8e83 100644 --- a/lib/pbench/client/__init__.py +++ b/lib/pbench/client/__init__.py @@ -77,6 +77,7 @@ def __init__(self, host: str): url = f"{self.scheme}://{self.host}" self.url = url self.username: Optional[str] = None + self.api_key: Optional[str] = None self.auth_token: Optional[str] = None self.session: Optional[requests.Session] = None self.endpoints: Optional[JSONOBJECT] = None @@ -101,8 +102,9 @@ def _headers( Case-insensitive header dictionary """ headers = CaseInsensitiveDict() - if self.auth_token: - headers["authorization"] = f"Bearer {self.auth_token}" + token = self.api_key if self.api_key else self.auth_token + if token: + headers["authorization"] = f"Bearer {token}" if user_headers: headers.update(user_headers) return headers @@ -336,6 +338,25 @@ def login(self, user: str, password: str): ) self.username = user self.auth_token = response["access_token"] + self.api_key = None + + def create_api_key(self): + """Create an API key from the current authenticated user + + Creating an API key will cause the new key to be used instead of a + normal login auth_token until the API key is removed. + """ + response = self.post(api=API.KEY) + self.api_key = response.json()["api_key"] + assert self.api_key, f"API key creation failed, {response.json()}" + + def remove_api_key(self): + """Remove the session's API key + + NOTE: when we support `DELETE /api/v1/key/{key}` this should delete the + key from the server. + """ + self.api_key = None def upload(self, tarball: Path, **kwargs) -> requests.Response: """Upload a tarball to the server. @@ -427,7 +448,9 @@ def get_list(self, **kwargs) -> Iterator[Dataset]: args = kwargs.copy() if "limit" not in args: args["limit"] = self.DEFAULT_PAGE_SIZE - json = self.get(api=API.DATASETS_LIST, params=args).json() + response = self.get(api=API.DATASETS_LIST, params=args, raise_error=False) + json = response.json() + assert response.ok, f"GET failed with {json['message']}" while True: for d in json["results"]: yield Dataset(d) diff --git a/lib/pbench/client/types.py b/lib/pbench/client/types.py index b3e865548d..d43c06fd13 100644 --- a/lib/pbench/client/types.py +++ b/lib/pbench/client/types.py @@ -49,6 +49,14 @@ def __getitem__(self, key: str) -> Any: """ return self.json[key] + def __repr__(self) -> str: + """Represent by returning the JSON representation""" + return repr(self.json) + + def __str__(self) -> str: + """Stringify by returning the stringified JSON""" + return str(self.json) + class Dataset(JSONMap): @staticmethod @@ -67,3 +75,7 @@ def md5(tarball: Path) -> str: """ md5_file = Path(f"{str(tarball)}.md5") return md5_file.read_text().split()[0] + + def __str__(self) -> str: + """Identify the dataset by ID and name""" + return f"Dataset({self.resource_id}, {self.name!r})" diff --git a/lib/pbench/server/database/alembic.migration b/lib/pbench/server/database/alembic.migration index 473f38e6fc..43d552590b 100755 --- a/lib/pbench/server/database/alembic.migration +++ b/lib/pbench/server/database/alembic.migration @@ -56,6 +56,9 @@ elif [[ "${1}" == "create" ]]; then # We have been asked to auto-generate a migration based on the existing # model compared against the most recent migration "head". alembic revision --autogenerate +elif [[ "${1}" == "show" ]]; then + alembic heads + alembic history else printf "Unsupported operation requested, '%s'\n" "${1}" >&2 exit 1 diff --git a/lib/pbench/test/functional/server/test_put.py b/lib/pbench/test/functional/server/test_put.py index f868c3ee30..78e04f62e9 100644 --- a/lib/pbench/test/functional/server/test_put.py +++ b/lib/pbench/test/functional/server/test_put.py @@ -354,6 +354,28 @@ def test_list_anonymous(self, server_client: PbenchServerClient): assert count > 1 + @pytest.mark.dependency(name="list_api_key", depends=["upload"], scope="session") + def test_list_api_key(self, server_client: PbenchServerClient, login_user): + """List "my" datasets using an API key. + + We should see all datasets owned by the tester account, both private + and public. That is, using the API key is the same as using the normal + auth token. + """ + server_client.create_api_key() + assert server_client.api_key, "No API key was set on the session" + datasets = server_client.get_list(mine="true") + + expected = [ + {"resource_id": Dataset.md5(f), "name": Dataset.stem(f), "metadata": {}} + for f in TARBALL_DIR.glob("*.tar.xz") + ] + expected.sort(key=lambda e: e["resource_id"]) + actual = [d.json for d in datasets] + assert expected == actual + server_client.remove_api_key() + assert not server_client.api_key, "API key was not removed as expected" + @pytest.mark.dependency(name="list_or", depends=["upload"], scope="session") def test_list_filter_or(self, server_client: PbenchServerClient, login_user): """Check a simple OR filter list.