diff --git a/Dockerfile b/Dockerfile index 795bb51..f60fa33 100644 --- a/Dockerfile +++ b/Dockerfile @@ -12,4 +12,4 @@ COPY ./vocab /usr/src/vocab # NB_API_PORT, representing the port on which the API will be exposed, # is an environment variable that will always have a default value of 8000 when building the image # but can be overridden when running the container. -ENTRYPOINT uvicorn app.main:app --proxy-headers --host 0.0.0.0 --port ${NB_API_PORT:-8000} +ENTRYPOINT uvicorn app.main:app --proxy-headers --forwarded-allow-ips=* --host 0.0.0.0 --port ${NB_API_PORT:-8000} diff --git a/README.md b/README.md index b68931c..88c9592 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,8 @@ Please refer to our [**official documentation**](https://neurobagel.org/api/) fo ## Quickstart The API is hosted at https://api.neurobagel.org/ and interfaces with Neurobagel's graph database. Queries of the graph can be run using the `/query` route. -Example: **I want to query for only female participants in the graph.** The URL for such a query would be https://api.neurobagel.org/query/?sex=snomed:248152002, where `snomed:248152002` is a [controlled term from the SNOMED CT database](http://purl.bioontology.org/ontology/SNOMEDCT/248152002) corresponding to female sex. +Example: **I want to query for only female participants in the graph.** +The URL for such a query would be https://api.neurobagel.org/query?sex=snomed:248152002, where `snomed:248152002` is a [controlled term from the SNOMED CT database](http://purl.bioontology.org/ontology/SNOMEDCT/248152002) corresponding to female sex. Interactive documentation for the API is available at https://api.neurobagel.org/docs. @@ -118,7 +119,7 @@ docker run -d --name=api -p ${NB_API_PORT_HOST}:${NB_API_PORT} --env-file=.env n By default, after running the above steps, the API should be served at localhost, http://127.0.0.1:8000/query, on the machine where you launched the Dockerized app. To check that the API is running and can access the knowledge graph as expected, you can navigate to the interactive API docs in your local browser (http://127.0.0.1:8000/docs) and enter a sample query, or send an HTTP request in your terminal using `curl`: ``` bash # example: query for female subjects in graph -curl -L http://127.0.0.1:8000/query/?sex=snomed:248152002 +curl http://127.0.0.1:8000/query?sex=snomed:248152002 ``` The response should be a list of dictionaries containing info about datasets with participants matching the query. diff --git a/app/api/routers/query.py b/app/api/routers/query.py index 539b27a..f746b7b 100644 --- a/app/api/routers/query.py +++ b/app/api/routers/query.py @@ -23,7 +23,12 @@ ) -@router.get("/", response_model=List[CohortQueryResponse]) +# We (unconventionally) use an "" path prefix here because we have globally disabled +# redirection of trailing slashes in the main app file. We use an empty string here +# to ensure that a request without a trailing slash (e.g., to /query instead of /query/) +# is correctly routed to this endpoint. +# For more context, see https://github.com/neurobagel/api/issues/327. +@router.get("", response_model=List[CohortQueryResponse]) async def get_query( query: QueryModel = Depends(QueryModel), token: str | None = Depends(oauth2_scheme), diff --git a/app/main.py b/app/main.py index c4a88cd..5d0ceb3 100644 --- a/app/main.py +++ b/app/main.py @@ -16,7 +16,10 @@ from .api.security import check_client_id app = FastAPI( - default_response_class=ORJSONResponse, docs_url=None, redoc_url=None + default_response_class=ORJSONResponse, + docs_url=None, + redoc_url=None, + redirect_slashes=False, ) favicon_url = "https://raw.githubusercontent.com/neurobagel/documentation/main/docs/imgs/logo/neurobagel_favicon.png" diff --git a/tests/conftest.py b/tests/conftest.py index 5d03c2d..dd92310 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,6 +11,12 @@ def test_app(): yield client +@pytest.fixture +def enable_auth(monkeypatch): + """Enable the authentication requirement for the API.""" + monkeypatch.setattr("app.api.security.AUTH_ENABLED", True) + + @pytest.fixture def disable_auth(monkeypatch): """ diff --git a/tests/test_app_events.py b/tests/test_app_events.py index eaaf001..943e798 100644 --- a/tests/test_app_events.py +++ b/tests/test_app_events.py @@ -39,7 +39,7 @@ def mock_httpx_post(**kwargs): return httpx.Response(status_code=401) monkeypatch.setattr(httpx, "post", mock_httpx_post) - response = test_app.get("/query/", headers=mock_auth_header) + response = test_app.get("/query", headers=mock_auth_header) assert response.status_code == 401 diff --git a/tests/test_attributes.py b/tests/test_attributes.py index 3be37a2..3cde96f 100644 --- a/tests/test_attributes.py +++ b/tests/test_attributes.py @@ -7,16 +7,6 @@ from app.api import utility as util -def test_root(test_app): - """Given a GET request to the root endpoint, Check for 200 status and expected content.""" - - response = test_app.get("/") - - assert response.status_code == 200 - assert "Welcome to the Neurobagel REST API!" in response.text - assert 'documentation' in response.text - - @pytest.mark.filterwarnings("ignore:.*NB_API_ALLOWED_ORIGINS") @pytest.mark.parametrize( "valid_data_element_URI", diff --git a/tests/test_query.py b/tests/test_query.py index 752b5a3..2e92ec0 100644 --- a/tests/test_query.py +++ b/tests/test_query.py @@ -6,6 +6,8 @@ import app.api.utility as util from app.api import crud +ROUTE = "/query" + def test_get_subjects_by_query(monkeypatch): """Test that graph results for dataset size queries are correctly parsed into a dictionary.""" @@ -68,7 +70,7 @@ def test_null_modalities( crud, "query_matching_dataset_sizes", mock_query_matching_dataset_sizes ) - response = test_app.get("/query/", headers=mock_auth_header) + response = test_app.get(ROUTE, headers=mock_auth_header) assert response.json()[0]["image_modals"] == [ "http://purl.org/nidash/nidm#T1Weighted" ] @@ -84,7 +86,7 @@ def test_get_all( """Given no input for any query parameters, returns a 200 status code and a non-empty list of results (should correspond to all subjects in graph).""" monkeypatch.setattr(crud, "get", mock_successful_get) - response = test_app.get("/query/", headers=mock_auth_header) + response = test_app.get(ROUTE, headers=mock_auth_header) assert response.status_code == 200 assert response.json() != [] @@ -106,7 +108,7 @@ def test_get_valid_age_range( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get( - f"/query/?min_age={valid_min_age}&max_age={valid_max_age}", + f"{ROUTE}?min_age={valid_min_age}&max_age={valid_max_age}", headers=mock_auth_header, ) assert response.status_code == 200 @@ -128,7 +130,7 @@ def test_get_valid_age_single_bound( """Given only a valid lower/upper age bound, returns a 200 status code and a non-empty list of results.""" monkeypatch.setattr(crud, "get", mock_successful_get) - response = test_app.get(f"/query/?{age_keyval}", headers=mock_auth_header) + response = test_app.get(f"{ROUTE}?{age_keyval}", headers=mock_auth_header) assert response.status_code == 200 assert response.json() != [] @@ -155,7 +157,7 @@ def test_get_invalid_age( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - f"/query/?min_age={invalid_min_age}&max_age={invalid_max_age}", + f"{ROUTE}?min_age={invalid_min_age}&max_age={invalid_max_age}", headers=mock_auth_header, ) assert response.status_code == 422 @@ -177,7 +179,7 @@ def test_get_valid_sex( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get( - f"/query/?sex={valid_sex}", headers=mock_auth_header + f"{ROUTE}?sex={valid_sex}", headers=mock_auth_header ) assert response.status_code == 200 assert response.json() != [] @@ -190,7 +192,7 @@ def test_get_invalid_sex( """Given an invalid sex string, returns a 422 status code.""" monkeypatch.setattr(crud, "get", mock_get) - response = test_app.get("/query/?sex=apple", headers=mock_auth_header) + response = test_app.get(f"{ROUTE}?sex=apple", headers=mock_auth_header) assert response.status_code == 422 @@ -209,7 +211,7 @@ def test_get_valid_diagnosis( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get( - f"/query/?diagnosis={valid_diagnosis}", headers=mock_auth_header + f"{ROUTE}?diagnosis={valid_diagnosis}", headers=mock_auth_header ) assert response.status_code == 200 assert response.json() != [] @@ -231,7 +233,7 @@ def test_get_invalid_diagnosis( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - f"/query/?diagnosis={invalid_diagnosis}", headers=mock_auth_header + f"{ROUTE}?diagnosis={invalid_diagnosis}", headers=mock_auth_header ) assert response.status_code == 422 @@ -249,7 +251,7 @@ def test_get_valid_iscontrol( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get( - f"/query/?is_control={valid_iscontrol}", headers=mock_auth_header + f"{ROUTE}?is_control={valid_iscontrol}", headers=mock_auth_header ) assert response.status_code == 200 assert response.json() != [] @@ -263,7 +265,7 @@ def test_get_invalid_iscontrol( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - "/query/?is_control=apple", headers=mock_auth_header + f"{ROUTE}?is_control=apple", headers=mock_auth_header ) assert response.status_code == 422 @@ -276,7 +278,7 @@ def test_get_invalid_control_diagnosis_pair( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - "/query/?diagnosis=snomed:35489007&is_control=True", + f"{ROUTE}?diagnosis=snomed:35489007&is_control=True", headers=mock_auth_header, ) assert response.status_code == 422 @@ -305,7 +307,7 @@ def test_get_valid_min_num_sessions( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get( - f"/query/?{session_param}={valid_min_num_sessions}", + f"{ROUTE}?{session_param}={valid_min_num_sessions}", headers=mock_auth_header, ) assert response.status_code == 200 @@ -331,7 +333,7 @@ def test_get_invalid_min_num_sessions( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - f"/query/?{session_param}={invalid_min_num_sessions}", + f"{ROUTE}?{session_param}={invalid_min_num_sessions}", headers=mock_auth_header, ) response.status_code = 422 @@ -348,7 +350,7 @@ def test_get_valid_assessment( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get( - "/query/?assessment=nb:cogAtlas-1234", headers=mock_auth_header + f"{ROUTE}?assessment=nb:cogAtlas-1234", headers=mock_auth_header ) assert response.status_code == 200 assert response.json() != [] @@ -370,7 +372,7 @@ def test_get_invalid_assessment( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - f"/query/?assessment={invalid_assessment}", headers=mock_auth_header + f"{ROUTE}?assessment={invalid_assessment}", headers=mock_auth_header ) assert response.status_code == 422 @@ -397,7 +399,7 @@ def test_get_valid_available_image_modal( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get( - f"/query/?image_modal={valid_available_image_modal}", + f"{ROUTE}?image_modal={valid_available_image_modal}", headers=mock_auth_header, ) assert response.status_code == 200 @@ -421,7 +423,7 @@ def test_get_valid_unavailable_image_modal( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - f"/query/?image_modal={valid_unavailable_image_modal}", + f"{ROUTE}?image_modal={valid_unavailable_image_modal}", headers=mock_auth_header, ) @@ -445,7 +447,7 @@ def test_get_invalid_image_modal( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - f"/query/?image_modal={invalid_image_modal}", headers=mock_auth_header + f"{ROUTE}?image_modal={invalid_image_modal}", headers=mock_auth_header ) assert response.status_code == 422 @@ -469,7 +471,7 @@ def test_get_undefined_prefix_image_modal( monkeypatch.setattr(crud, "get", mock_get_with_exception) response = test_app.get( - f"/query/?image_modal={undefined_prefix_image_modal}", + f"{ROUTE}?image_modal={undefined_prefix_image_modal}", headers=mock_auth_header, ) assert response.status_code == 500 @@ -491,7 +493,7 @@ def test_aggregate_query_response_structure( crud, "query_matching_dataset_sizes", mock_query_matching_dataset_sizes ) - response = test_app.get("/query/", headers=mock_auth_header) + response = test_app.get(ROUTE, headers=mock_auth_header) assert all( dataset["subject_data"] == "protected" for dataset in response.json() ) @@ -507,5 +509,5 @@ def test_query_without_token_succeeds_when_auth_disabled( Test that when authentication is disabled, a request to the /query route without a token succeeds. """ monkeypatch.setattr(crud, "get", mock_successful_get) - response = test_app.get("/query/") + response = test_app.get(ROUTE) assert response.status_code == 200 diff --git a/tests/test_routing.py b/tests/test_routing.py new file mode 100644 index 0000000..7b03001 --- /dev/null +++ b/tests/test_routing.py @@ -0,0 +1,45 @@ +import pytest + +from app.api import crud + + +@pytest.mark.parametrize( + "root_path", + ["/", ""], +) +def test_root(test_app, root_path): + """Given a GET request to the root endpoint, Check for 200 status and expected content.""" + + response = test_app.get(root_path, follow_redirects=False) + + assert response.status_code == 200 + assert "Welcome to the Neurobagel REST API!" in response.text + assert 'documentation' in response.text + + +@pytest.mark.parametrize( + "valid_route", + ["/query", "/query?min_age=20"], +) +def test_request_without_trailing_slash_not_redirected( + test_app, monkeypatch, mock_successful_get, disable_auth, valid_route +): + """Test that a request to a route without a / is not redirected to have a trailing slash.""" + monkeypatch.setattr(crud, "get", mock_successful_get) + response = test_app.get(valid_route, follow_redirects=False) + assert response.status_code == 200 + + +@pytest.mark.parametrize( + "invalid_route", + ["/query/", "/query/?min_age=20", "/attributes/nb:SomeClass/"], +) +def test_request_with_trailing_slash_not_redirected( + test_app, disable_auth, invalid_route +): + """ + Test that a request to routes including a trailing slash, where none is expected, + is *not* redirected to exclude the slash, and returns a 404. + """ + response = test_app.get(invalid_route) + assert response.status_code == 404 diff --git a/tests/test_security.py b/tests/test_security.py index 9a46ef8..57cbe3e 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -6,14 +6,13 @@ @pytest.mark.filterwarnings("ignore:.*NB_API_ALLOWED_ORIGINS") def test_missing_client_id_raises_error_when_auth_enabled( - monkeypatch, test_app, set_test_credentials + monkeypatch, test_app, set_test_credentials, enable_auth ): """Test that a missing client ID raises an error on startup when authentication is enabled.""" # We're using what should be default values of CLIENT_ID and AUTH_ENABLED here # (if the corresponding environment variables are unset), # but we set the values explicitly here for clarity monkeypatch.setattr("app.api.security.CLIENT_ID", None) - monkeypatch.setattr("app.api.security.AUTH_ENABLED", True) with pytest.raises(ValueError) as exc_info: with test_app: @@ -52,17 +51,20 @@ def test_invalid_token_raises_error(invalid_token): [{}, {"Authorization": ""}, {"badheader": "badvalue"}], ) def test_query_with_malformed_auth_header_fails( - test_app, set_mock_verify_token, invalid_auth_header, monkeypatch + test_app, + set_mock_verify_token, + enable_auth, + invalid_auth_header, + monkeypatch, ): """ - Test that when authentication is enabled, a request to the /query route with - a missing or malformed authorization header fails. + Test that when authentication is enabled, a request to the /query route with a + missing or malformed authorization header fails. """ monkeypatch.setattr("app.api.security.CLIENT_ID", "foo.id") - monkeypatch.setattr("app.api.security.AUTH_ENABLED", True) response = test_app.get( - "/query/", + "/query", headers=invalid_auth_header, )