Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions app/api/env_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ class Settings(BaseSettings):
description="The base URL path prefix for the API. When deployed behind a reverse proxy, set this to the subpath at which the app is mounted (if any), "
"and configure the proxy to strip this prefix from incoming requests.",
)
allowed_origins: str | None = Field(
alias="NB_API_ALLOWED_ORIGINS", default=None
)
graph_username: str | None = Field(alias="NB_GRAPH_USERNAME", default=None)
graph_password: str | None = Field(alias="NB_GRAPH_PASSWORD", default=None)
graph_address: str = Field(alias="NB_GRAPH_ADDRESS", default="127.0.0.1")
Expand Down
5 changes: 0 additions & 5 deletions app/api/utility.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ def load_json(path: Path) -> dict:
return json.load(f)


def parse_origins_as_list(allowed_origins: str | None) -> list:
"""Returns user-defined allowed origins as a list."""
return list(allowed_origins.split(" ")) if allowed_origins else []


def create_gh_raw_content_url(repo: str, content_path: str) -> str:
"""
Create a raw content URL for a given path in a specific GitHub repository.
Expand Down
21 changes: 0 additions & 21 deletions app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import uvicorn
from fastapi import FastAPI, Request
from fastapi.middleware.cors import CORSMiddleware
from fastapi.openapi.docs import get_redoc_html, get_swagger_ui_html
from fastapi.responses import HTMLResponse, ORJSONResponse, RedirectResponse

Expand Down Expand Up @@ -60,18 +59,6 @@ def validate_environment_variables():
f"The application was launched but could not find the {Settings.model_fields['graph_username'].alias} and / or {Settings.model_fields['graph_password'].alias} environment variables.",
)

if settings.allowed_origins is None:
logger.warning(
f"The API was launched without providing any values for the {Settings.model_fields['allowed_origins'].alias} environment "
"variable. "
"This means that the API will only be accessible from the same origin it is hosted from: "
"https://developer.mozilla.org/en-US/docs/Web/Security/Same-origin_policy. "
"If you want to access the API from tools hosted at other origins such as the Neurobagel query tool, "
f"explicitly set the value of {Settings.model_fields['allowed_origins'].alias} to the origin(s) of these tools (e.g. "
"http://localhost:3000). "
"Multiple allowed origins should be separated with spaces in a single string enclosed in quotes."
)

available_configs = fetch_available_community_config_names()
if settings.config not in available_configs:
log_and_raise_error(
Expand Down Expand Up @@ -226,14 +213,6 @@ async def lifespan(app: FastAPI):
redirect_slashes=False,
)

app.add_middleware(
CORSMiddleware,
allow_origins=util.parse_origins_as_list(settings.allowed_origins),
allow_credentials=True,
allow_methods=["*"],
allow_headers=["*"],
)


@app.get("/", response_class=HTMLResponse)
Comment thread
AviJxn marked this conversation as resolved.
def root(request: Request):
Expand Down
64 changes: 1 addition & 63 deletions tests/test_app_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

from app import main
from app.api import env_settings
from app.api import utility as util
from app.main import settings


Expand Down Expand Up @@ -35,68 +34,7 @@ def test_start_app_without_environment_vars_fails(
assert expected_msg in str(e_info.value)


def test_app_with_unset_allowed_origins(
test_app,
disable_auth,
set_temp_datasets_metadata_file,
monkeypatch,
caplog,
):
"""Tests that when the environment variable for allowed origins has not been set, a warning is raised and the app uses an empty list."""
monkeypatch.setattr(settings, "allowed_origins", None)
expected_warning = "API was launched without providing any values for the NB_API_ALLOWED_ORIGINS environment variable"

with test_app:
pass

warnings = [
record
for record in caplog.records
if record.levelno == logging.WARNING
]

assert len(warnings) == 1
assert expected_warning in warnings[0].getMessage()
assert util.parse_origins_as_list(settings.allowed_origins) == []


@pytest.mark.parametrize(
"allowed_origins, parsed_origins",
[
("http://localhost:3000", ["http://localhost:3000"]),
(
"http://localhost:3000 https://localhost:3000",
["http://localhost:3000", "https://localhost:3000"],
),
(
" http://localhost:3000 https://localhost:3000 ",
["http://localhost:3000", "https://localhost:3000"],
),
],
)
def test_app_with_set_allowed_origins(
test_app,
monkeypatch,
allowed_origins,
parsed_origins,
disable_auth,
set_temp_datasets_metadata_file,
):
"""
Test that when the environment variable for allowed origins has been explicitly set,
the app correctly parses it into a list.
"""
monkeypatch.setattr(settings, "allowed_origins", allowed_origins)

with test_app:
pass

assert set(parsed_origins).issubset(
util.parse_origins_as_list(settings.allowed_origins)
)


def fetched_configs_includes_neurobagel(disable_auth):
def test_fetched_configs_includes_neurobagel(disable_auth):
"""Test that "Neurobagel" is included among the available configuration names fetched from GitHub."""
assert "Neurobagel" in main.fetch_available_community_config_names()

Expand Down
1 change: 0 additions & 1 deletion tests/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ def test_settings_read_correctly(monkeypatch):
assert settings.graph_db == "repositories/my_db"

# Check that set environment variables are read and typed correctly
assert settings.allowed_origins == "*"
assert settings.graph_username == "DBUSER"
assert settings.graph_password == "DBPASSWORD"
assert settings.graph_port == 7201
Expand Down