-
-
Notifications
You must be signed in to change notification settings - Fork 13
feat: Migrate from URI-encoded API keys to Authorization header #210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3159bbf
1d27dd1
72d17eb
5557fc0
b695e23
7decfe1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| docker/mysql/data | ||
| r/mysql/data | ||
|
|
||
| # Byte-compiled / optimized / DLL files | ||
| __pycache__/ | ||
|
|
||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,10 +15,11 @@ | |
| ids=["no authentication", "invalid key"], | ||
| ) | ||
| def test_dataset_tag_rejects_unauthorized(key: ApiKey, py_api: TestClient) -> None: | ||
| apikey = "" if key is None else f"?api_key={key}" | ||
| headers = {} if key is None else {"Authorization": key} | ||
| response = py_api.post( | ||
| f"/datasets/tag{apikey}", | ||
| "/datasets/tag", | ||
| json={"data_id": next(iter(constants.PRIVATE_DATASET_ID)), "tag": "test"}, | ||
| headers=headers, | ||
| ) | ||
| assert response.status_code == HTTPStatus.PRECONDITION_FAILED | ||
| assert response.json()["detail"] == {"code": "103", "message": "Authentication failed"} | ||
|
|
@@ -32,8 +33,9 @@ def test_dataset_tag_rejects_unauthorized(key: ApiKey, py_api: TestClient) -> No | |
| def test_dataset_tag(key: ApiKey, expdb_test: Connection, py_api: TestClient) -> None: | ||
| dataset_id, tag = next(iter(constants.PRIVATE_DATASET_ID)), "test" | ||
| response = py_api.post( | ||
| f"/datasets/tag?api_key={key}", | ||
| "/datasets/tag", | ||
| json={"data_id": dataset_id, "tag": tag}, | ||
| headers={"Authorization": key}, | ||
| ) | ||
| assert response.status_code == HTTPStatus.OK | ||
| assert response.json() == {"data_tag": {"id": str(dataset_id), "tag": [tag]}} | ||
|
|
@@ -45,8 +47,9 @@ def test_dataset_tag(key: ApiKey, expdb_test: Connection, py_api: TestClient) -> | |
| def test_dataset_tag_returns_existing_tags(py_api: TestClient) -> None: | ||
| dataset_id, tag = 1, "test" | ||
| response = py_api.post( | ||
| f"/datasets/tag?api_key={ApiKey.ADMIN}", | ||
| "/datasets/tag", | ||
| json={"data_id": dataset_id, "tag": tag}, | ||
| headers={"Authorization": ApiKey.ADMIN}, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Test for missing Authorization header is present, but consider testing for extra/invalid headers. Consider adding a test where the Authorization header contains an unexpected or malformed value to verify the endpoint correctly rejects such requests. |
||
| ) | ||
| assert response.status_code == HTTPStatus.OK | ||
| assert response.json() == {"data_tag": {"id": str(dataset_id), "tag": ["study_14", tag]}} | ||
|
|
@@ -55,8 +58,9 @@ def test_dataset_tag_returns_existing_tags(py_api: TestClient) -> None: | |
| def test_dataset_tag_fails_if_tag_exists(py_api: TestClient) -> None: | ||
| dataset_id, tag = 1, "study_14" # Dataset 1 already is tagged with 'study_14' | ||
| response = py_api.post( | ||
| f"/datasets/tag?api_key={ApiKey.ADMIN}", | ||
| "/datasets/tag", | ||
| json={"data_id": dataset_id, "tag": tag}, | ||
| headers={"Authorization": ApiKey.ADMIN}, | ||
| ) | ||
| assert response.status_code == HTTPStatus.INTERNAL_SERVER_ERROR | ||
| expected = { | ||
|
|
@@ -79,8 +83,9 @@ def test_dataset_tag_invalid_tag_is_rejected( | |
| py_api: TestClient, | ||
| ) -> None: | ||
| new = py_api.post( | ||
| f"/datasets/tag?api_key{ApiKey.ADMIN}", | ||
| "/datasets/tag", | ||
| json={"data_id": 1, "tag": tag}, | ||
| headers={"Authorization": ApiKey.ADMIN}, | ||
| ) | ||
|
|
||
| assert new.status_code == HTTPStatus.UNPROCESSABLE_ENTITY | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -53,10 +53,11 @@ def test_list_filter_active(status: str, amount: int, py_api: TestClient) -> Non | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_list_accounts_privacy(api_key: ApiKey | None, amount: int, py_api: TestClient) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key = f"?api_key={api_key}" if api_key else "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers = {"Authorization": api_key} if api_key else {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = py_api.post( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"/datasets/list{key}", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "/datasets/list", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| json={"status": "all", "pagination": {"limit": 1000}}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers=headers, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert response.status_code == HTTPStatus.OK, response.json() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert len(response.json()) == amount | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
55
to
63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Consider adding tests for malformed or missing Authorization headers. Tests should also cover cases like empty, incorrectly formatted, or multiple Authorization headers to verify error handling and security.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -69,8 +70,9 @@ def test_list_accounts_privacy(api_key: ApiKey | None, amount: int, py_api: Test | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_list_data_name_present(name: str, count: int, py_api: TestClient) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # The second iris dataset is private, so we need to authenticate. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = py_api.post( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"/datasets/list?api_key={ApiKey.ADMIN}", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "/datasets/list", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| json={"status": "all", "data_name": name}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers={"Authorization": ApiKey.ADMIN}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert response.status_code == HTTPStatus.OK | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| datasets = response.json() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -84,8 +86,9 @@ def test_list_data_name_present(name: str, count: int, py_api: TestClient) -> No | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_list_data_name_absent(name: str, py_api: TestClient) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = py_api.post( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"/datasets/list?api_key={ApiKey.ADMIN}", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "/datasets/list", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| json={"status": "all", "data_name": name}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers={"Authorization": ApiKey.ADMIN}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _assert_empty_result(response) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -123,8 +126,9 @@ def test_list_pagination(limit: int | None, offset: int | None, py_api: TestClie | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_list_data_version(version: int, count: int, py_api: TestClient) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = py_api.post( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"/datasets/list?api_key={ApiKey.ADMIN}", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "/datasets/list", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| json={"status": "all", "data_version": version}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers={"Authorization": ApiKey.ADMIN}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert response.status_code == HTTPStatus.OK | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| datasets = response.json() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -134,8 +138,9 @@ def test_list_data_version(version: int, count: int, py_api: TestClient) -> None | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_list_data_version_no_result(py_api: TestClient) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = py_api.post( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"/datasets/list?api_key={ApiKey.ADMIN}", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "/datasets/list", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| json={"status": "all", "data_version": 4}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers={"Authorization": ApiKey.ADMIN}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _assert_empty_result(response) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -150,8 +155,9 @@ def test_list_data_version_no_result(py_api: TestClient) -> None: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_list_uploader(user_id: int, count: int, key: str, py_api: TestClient) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = py_api.post( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"/datasets/list?api_key={key}", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "/datasets/list", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| json={"status": "all", "uploader": user_id}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers={"Authorization": key}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # The dataset of user 16 is private, so can not be retrieved by other users. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| owner_user_id = 16 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -255,7 +261,7 @@ def test_list_data_identical( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return hypothesis.reject() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| api_key = kwargs.pop("api_key") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| api_key_query = f"?api_key={api_key}" if api_key else "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers = {"Authorization": api_key} if api_key else {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Pagination parameters are nested in the new query style | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # The old style has no `limit` by default, so we mimic this with a high default | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -264,8 +270,9 @@ def test_list_data_identical( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| new_style["pagination"]["offset"] = offset | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = py_api.post( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"/datasets/list{api_key_query}", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "/datasets/list", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| json=new_style, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers=headers, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # old style `/data/filter` encodes all filters as a path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -274,6 +281,7 @@ def test_list_data_identical( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for filter_, value in kwargs.items() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if value is not None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| api_key_query = f"?api_key={api_key}" if api_key else "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uri = "/data/list" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if query: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uri += f"/{'/'.join([str(v) for q in query for v in q])}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,8 +110,8 @@ def test_private_dataset_no_user_no_access( | |
| py_api: TestClient, | ||
| api_key: str | None, | ||
| ) -> None: | ||
| query = f"?api_key={api_key}" if api_key else "" | ||
| response = py_api.get(f"/datasets/130{query}") | ||
| headers = {"Authorization": api_key} if api_key else {} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Consider adding a test for both query parameter and header present. Add a test case with both the query parameter and Authorization header to confirm the correct precedence and maintain backward compatibility. |
||
| response = py_api.get("/datasets/130", headers=headers) | ||
|
|
||
| # New response is 403: Forbidden instead of 412: PRECONDITION FAILED | ||
| assert response.status_code == HTTPStatus.FORBIDDEN | ||
|
|
@@ -128,7 +128,7 @@ def test_private_dataset_owner_access( | |
| api_key: str, | ||
| ) -> None: | ||
| [private_dataset] = constants.PRIVATE_DATASET_ID | ||
| new_response = py_api.get(f"/datasets/{private_dataset}?api_key={api_key}") | ||
| new_response = py_api.get(f"/datasets/{private_dataset}", headers={"Authorization": api_key}) | ||
| old_response = php_api.get(f"/data/{private_dataset}?api_key={api_key}") | ||
| assert old_response.status_code == HTTPStatus.OK | ||
| assert old_response.status_code == new_response.status_code | ||
|
|
@@ -177,8 +177,9 @@ def test_dataset_tag_response_is_identical( | |
| ): | ||
| pytest.skip("Encountered Elastic Search error.") | ||
| new = py_api.post( | ||
| f"/datasets/tag?api_key={api_key}", | ||
| "/datasets/tag", | ||
| json={"data_id": dataset_id, "tag": tag}, | ||
| headers={"Authorization": api_key}, | ||
| ) | ||
|
|
||
| assert original.status_code == new.status_code, original.json() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Non-standard Authorization header usage without scheme prefix.
The implementation uses the
Authorizationheader to carry raw API keys without a scheme prefix (e.g.,Authorization: <api_key>). This deviates from RFC 7235 and common REST API practices, where the Authorization header should follow the formatAuthorization: <scheme> <credentials>(e.g.,Authorization: Bearer <token>orAuthorization: ApiKey <key>).Recommended alternatives:
X-API-KeyorX-Auth-TokenAuthorization: Bearer <api_key>orAuthorization: ApiKey <api_key>Using a non-standard format may cause confusion for API consumers and compatibility issues with HTTP clients, proxies, or API gateways that expect standard Authorization header formats.
Also applies to: 11-12
🌐 Web query:
💡 Result:
Short answer
Recommended minimal rules / best practices
References
🏁 Script executed:
Length of output: 976
🏁 Script executed:
Length of output: 872
🌐 Web query:
💡 Result:
Short answer
Minimal examples
from fastapi import Depends, FastAPI
from fastapi.security import APIKeyHeader
app = FastAPI()
api_key_header = APIKeyHeader(name="X-API-Key")
@app.get("/items/")
async def read_items(key: str = Depends(api_key_header)):
return {"key": key}
(returns the header string exactly as sent). [1]
from fastapi.security import HTTPBearer, HTTPAuthorizationCredentials
bearer = HTTPBearer()
@app.get("/secure/")
async def secure(creds: HTTPAuthorizationCredentials = Depends(bearer)):
# creds.scheme == "Bearer", creds.credentials == ""
return {"scheme": creds.scheme, "token": creds.credentials}
(HTTPBearer splits "Authorization: Bearer " for you). [2]
from fastapi.security.utils import get_authorization_scheme_param
authorization = request.headers.get("Authorization")
scheme, param = get_authorization_scheme_param(authorization)
if scheme.lower() == "bearer":
token = param
References
[1] FastAPI — APIKeyHeader docs.
[2] FastAPI — HTTP auth / HTTPAuthorizationCredentials (HTTPBearer) docs.
[3] Example usage of get_authorization_scheme_param in FastAPI (parsing Authorization header).
🌐 Web query:
💡 Result:
Short answer: use APIKeyHeader with name="Authorization" as a Security dependency (not plain Depends), then parse/strip the "Bearer " prefix yourself. Example:
Notes:
Sources: FastAPI security docs and community examples. [1] FastAPI security/APIKeyHeader reference. [2] StackOverflow discussion showing APIKeyHeader with Authorization.
Use HTTPBearer or custom header to enforce RFC 7235 Authorization header format.
The current implementation uses
APIKeyHeader(name="Authorization")which accepts raw API key values without enforcing a scheme prefix, violating RFC 7235. RFC 7235 requires Authorization headers to follow the formatAuthorization: <auth-scheme> <credentials>(e.g.,Bearer <token>orApiKey <key>). FastAPI's APIKeyHeader simply extracts the raw header value without scheme parsing or validation.Recommended fix:
HTTPBearer()dependency (automatically parses scheme and credentials)X-API-Keyinstead of Authorizationget_authorization_scheme_param()to extract and validate the scheme🤖 Prompt for AI Agents