From 13fe76f13fe6edf1a2f634cf7e5573c1a849780c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arturo=20Filast=C3=B2?= Date: Fri, 15 Mar 2024 07:23:57 +0100 Subject: [PATCH] Reach 99% code coverage The only missing lines are those related to setting up clickhouse and boto. We should eventually test these too inside of integration tests. --- ooniapi/services/ooniauth/pyproject.toml | 7 +- .../ooniauth/src/ooniauth/routers/v1.py | 39 ++-- ooniapi/services/ooniauth/tests/conftest.py | 96 +++++++--- ooniapi/services/ooniauth/tests/test_auth.py | 174 +++++++++++++----- ooniapi/services/ooniauth/tests/test_main.py | 55 ++++++ 5 files changed, 272 insertions(+), 99 deletions(-) create mode 100644 ooniapi/services/ooniauth/tests/test_main.py diff --git a/ooniapi/services/ooniauth/pyproject.toml b/ooniapi/services/ooniauth/pyproject.toml index c23c093e..8bd64c2a 100644 --- a/ooniapi/services/ooniauth/pyproject.toml +++ b/ooniapi/services/ooniauth/pyproject.toml @@ -85,7 +85,12 @@ check = "mypy --install-types --non-interactive {args:src/ooniauth tests}" source_pkgs = ["ooniauth", "tests"] branch = true parallel = true -omit = ["src/ooniauth/__about__.py", "src/ooniauth/common/*"] +omit = [ + "src/ooniauth/__about__.py", + "src/ooniauth/common/*", + # Ignored because these should be run manually on deployed instance + "tests/run_*", +] [tool.coverage.paths] ooniauth = ["src/ooniauth", "*/ooniauth/src/ooniauth"] diff --git a/ooniapi/services/ooniauth/src/ooniauth/routers/v1.py b/ooniapi/services/ooniauth/src/ooniauth/routers/v1.py index 7adf2dbc..20f8d17b 100644 --- a/ooniapi/services/ooniauth/src/ooniauth/routers/v1.py +++ b/ooniapi/services/ooniauth/src/ooniauth/routers/v1.py @@ -49,14 +49,10 @@ class UserRegister(BaseModel): min_length=5, max_length=255, ) - redirect_to: Optional[str] = Field(title="redirect to this URL", default="") + redirect_to: str = Field(title="redirect to this URL") @validator("redirect_to") def validate_redirect_to(cls, v): - # None is also a valid type - if v is None: - return v - u = urlparse(v) if u.scheme != "https": raise ValueError("Invalid URL") @@ -74,10 +70,7 @@ def validate_redirect_to(cls, v): return v -def format_login_url(redirect_to: Optional[str], registration_token: str) -> str: - if redirect_to is None: - return "" - +def format_login_url(redirect_to: str, registration_token: str) -> str: login_fqdm = urlparse(redirect_to).netloc e = urlencode(dict(token=registration_token)) return urlunsplit(("https", login_fqdm, "/login", e, "")) @@ -152,11 +145,11 @@ async def user_login( dec = decode_jwt( token=token, key=settings.jwt_encryption_key, audience="register" ) - except jwt.exceptions.MissingRequiredClaimError: - raise HTTPException(401, "Invalid token") - except jwt.exceptions.InvalidSignatureError: - raise HTTPException(401, "Invalid credential signature") - except jwt.exceptions.DecodeError: + except ( + jwt.exceptions.MissingRequiredClaimError, + jwt.exceptions.InvalidSignatureError, + jwt.exceptions.DecodeError, + ): raise HTTPException(401, "Invalid credentials") except jwt.exceptions.ExpiredSignatureError: raise HTTPException(401, "Expired token") @@ -205,9 +198,8 @@ async def user_refresh_token( authorization=authorization, jwt_encryption_key=settings.jwt_encryption_key ) - # @role_required already checked for expunged tokens - if not tok: - raise HTTPException(401, "Invalid credentials") + # @role_required already checked for validity of token + assert tok is not None newtoken = create_session_token( key=settings.jwt_encryption_key, @@ -239,12 +231,9 @@ async def get_account_metadata( schema: type: object """ - try: - tok = get_client_token( - authorization=authorization, jwt_encryption_key=settings.jwt_encryption_key - ) - if not tok: - raise HTTPException(401, "Invalid credentials") - return AccountMetadata(logged_in=True, role=tok["role"]) - except Exception: + tok = get_client_token( + authorization=authorization, jwt_encryption_key=settings.jwt_encryption_key + ) + if not tok: return AccountMetadata(logged_in=False, role="") + return AccountMetadata(logged_in=True, role=tok["role"]) diff --git a/ooniapi/services/ooniauth/tests/conftest.py b/ooniapi/services/ooniauth/tests/conftest.py index 0c4fd189..55c26588 100644 --- a/ooniapi/services/ooniauth/tests/conftest.py +++ b/ooniapi/services/ooniauth/tests/conftest.py @@ -9,6 +9,7 @@ from ooniauth.common.config import Settings from ooniauth.common.dependencies import get_settings from ooniauth.dependencies import get_ses_client, get_clickhouse_client +from ooniauth.utils import hash_email_address from ooniauth.main import app @@ -29,6 +30,41 @@ def client_with_bad_settings(): yield client +@pytest.fixture +def user_email(): + return "dev+useraccount@ooni.org" + + +@pytest.fixture +def admin_email(): + return "dev+adminaccount@ooni.org" + + +@pytest.fixture +def jwt_encryption_key(): + return "super_secure" + + +@pytest.fixture +def prometheus_password(): + return "super_secure" + + +@pytest.fixture +def account_id_hashing_key(): + return "super_secure" + + +@pytest.fixture +def email_source_address(): + return "admin+sourceemail@ooni.org" + + +@pytest.fixture +def valid_redirect_to_url(): + return "https://explorer.ooni.org" + + @pytest.fixture def mock_ses_client(): mock = MagicMock() @@ -37,40 +73,48 @@ def mock_ses_client(): @pytest.fixture -def client(mock_ses_client): +def mock_misconfigured_ses_client(): + mock = MagicMock() + mock.send_email.side_effect = Exception("failing to send an email") + app.dependency_overrides[get_ses_client] = lambda: mock + yield mock + + +@pytest.fixture +def client( + mock_ses_client, + admin_email, + jwt_encryption_key, + account_id_hashing_key, + prometheus_password, + email_source_address, +): app.dependency_overrides[get_settings] = make_override_get_settings( - jwt_encryption_key="super_secure", - prometheus_metrics_password="super_secure", - email_source_address="admin+sourceemail@ooni.org", + jwt_encryption_key=jwt_encryption_key, + prometheus_metrics_password=prometheus_password, + email_source_address=email_source_address, + account_id_hashing_key=account_id_hashing_key, + aws_access_key_id="ITSCHANGED", + aws_secret_access_key="ITSCHANGED", ) mock_clickhouse = MagicMock() mock_clickhouse.execute = MagicMock() # rows, coldata = q # coldata = [("name", "type")] - mock_clickhouse.execute.return_value = ( - [("user",)], - [("role", "String")], - ) + def mock_execute(query, query_params, with_column_types, settings): + assert with_column_types == True + print(settings) + assert query.startswith("SELECT role FROM") + if query_params["account_id"] == hash_email_address( + email_address=admin_email, key=account_id_hashing_key + ): + return [("admin",)], [("role", "String")] + + return [("user",)], [("role", "String")] + + mock_clickhouse.execute = mock_execute app.dependency_overrides[get_clickhouse_client] = lambda: mock_clickhouse client = TestClient(app) yield client - - -def create_jwt(payload: dict) -> str: - return jwt.encode(payload, "super_secure", algorithm="HS256") - - -def create_session_token(account_id: str, role: str) -> str: - now = int(time.time()) - payload = { - "nbf": now, - "iat": now, - "exp": now + 10 * 86400, - "aud": "user_auth", - "account_id": account_id, - "login_time": None, - "role": role, - } - return create_jwt(payload) diff --git a/ooniapi/services/ooniauth/tests/test_auth.py b/ooniapi/services/ooniauth/tests/test_auth.py index 4c84a6fe..e8768fce 100644 --- a/ooniapi/services/ooniauth/tests/test_auth.py +++ b/ooniapi/services/ooniauth/tests/test_auth.py @@ -16,43 +16,8 @@ from freezegun import freeze_time -admin_e = "integtest@openobservatory.org" -user_e = "admin+dev@ooni.org" - - -def test_login_user_bogus_token(client): - r = client.get("/api/v1/user_login?k=BOGUS") - assert r.status_code == 401 - # Note the key changed from "error" to "detail" - assert r.json() == {"detail": "Invalid credentials"} - - -def test_user_register_non_valid_email(client): - d = dict( - email_address="nick@localhost", redirect_to="https://explorer.ooni.org" - ) # no FQDN - r = client.post("/api/v1/user_register", json=d) - assert r.status_code == 422 - j = r.json() - assert j["detail"][0]["loc"] == ["body", "email_address"] - - -def test_user_register_non_valid_redirect(client): - d = dict( - email_address="nick@a.org", redirect_to="https://BOGUS.ooni.org" - ) # bogus fqdn - r = client.post("/api/v1/user_register", json=d) - assert r.status_code == 422 - - -def test_user_register_missing_redirect(client): - d = dict(email_address="nick@a.org") - r = client.post("/api/v1/user_register", json=d) - assert r.status_code == 200 - - -def _register_and_login(client, email_address, mock_ses_client): - d = dict(email_address=email_address, redirect_to="https://explorer.ooni.org") +def register(client, email_address, mock_ses_client, valid_redirect_to_url): + d = dict(email_address=email_address, redirect_to=valid_redirect_to_url) r = client.post("/api/v1/user_register", json=d) j = r.json() assert r.status_code == 200 @@ -83,34 +48,149 @@ def handle_starttag(self, tag, attrs): login_link = parser.links[0] token = parse_qs(urlparse(login_link).query)["token"][0] assert len(token) > 300 + return token + +def register_and_login(client, email_address, mock_ses_client, valid_redirect_to_url): + token = register( + client, + email_address=email_address, + mock_ses_client=mock_ses_client, + valid_redirect_to_url=valid_redirect_to_url, + ) r = client.get(f"/api/v1/user_login?k={token}") j = r.json() assert r.status_code == 200, j - assert j["redirect_to"] == "https://explorer.ooni.org" + assert j["redirect_to"] == valid_redirect_to_url assert "bearer" in j return {"Authorization": "Bearer " + j["bearer"]} -def test_user_register_and_logout(client, mock_ses_client): - j = client.get("/api/_/account_metadata").json() - assert j["logged_in"] == False - h = _register_and_login(client, user_e, mock_ses_client) +def test_login_user_bogus_token(client): + r = client.get("/api/v1/user_login?k=BOGUS") + assert r.status_code == 401 + # Note the key changed from "error" to "detail" + assert r.json() == {"detail": "Invalid credentials"} + + +def test_user_register_non_valid_email(client, valid_redirect_to_url): + d = dict( + email_address="nick@localhost", redirect_to=valid_redirect_to_url + ) # no FQDN + r = client.post("/api/v1/user_register", json=d) + assert r.status_code == 422 + j = r.json() + assert j["detail"][0]["loc"] == ["body", "email_address"] + + +def test_user_register_non_valid_redirect(client, valid_redirect_to_url): + d = dict( + email_address="nick@a.org", redirect_to="https://BOGUS.example.com" + ) # bogus fqdn + r = client.post("/api/v1/user_register", json=d) + assert r.status_code == 422 + + d = dict( + email_address="nick@a.org", + redirect_to=valid_redirect_to_url.replace("https", "http"), + ) # bogus fqdn + r = client.post("/api/v1/user_register", json=d) + assert r.status_code == 422 + + +def test_user_register_missing_redirect(client, valid_redirect_to_url): + d = dict(email_address="nick@a.org", redirect_to=valid_redirect_to_url) + r = client.post("/api/v1/user_register", json=d) + assert r.status_code == 200 + + +def test_user_refresh(client, mock_ses_client, user_email, valid_redirect_to_url): + r = client.get( + "/api/v1/user_refresh_token", headers={"Authorization": "Bearer invalidtoken"} + ) + assert r.status_code != 200 + j = r.json() + print(j) + + h = register_and_login(client, user_email, mock_ses_client, valid_redirect_to_url) j = client.get("/api/_/account_metadata", headers=h).json() - assert j == {"logged_in": True, "role": "user"} # logged in - # simulate logout by not using the token in variable "h" + assert j["logged_in"] == True + assert j["role"] == "user" + + j = client.get("/api/v1/user_refresh_token", headers=h).json() + assert len(j["bearer"]) > 100 + + +def test_user_register_and_refresh( + client, mock_ses_client, user_email, valid_redirect_to_url +): j = client.get("/api/_/account_metadata").json() assert j["logged_in"] == False + h = register_and_login(client, user_email, mock_ses_client, valid_redirect_to_url) + j = client.get("/api/_/account_metadata", headers=h).json() + assert j["logged_in"] == True + assert j["role"] == "user" -def test_user_register_and_get_metadata(client, mock_ses_client): +def test_user_register_misconfigured_email( + client, mock_misconfigured_ses_client, user_email, valid_redirect_to_url +): + d = dict(email_address=user_email, redirect_to=valid_redirect_to_url) + r = client.post("/api/v1/user_register", json=d) + assert r.status_code != 200 + assert isinstance(mock_misconfigured_ses_client.send_email.side_effect, Exception) + + +def test_user_register_and_get_metadata( + client, mock_ses_client, user_email, valid_redirect_to_url +): r = client.get("/api/_/account_metadata") j = r.json() assert j["logged_in"] == False - h = _register_and_login(client, user_e, mock_ses_client) + h = register_and_login(client, user_email, mock_ses_client, valid_redirect_to_url) r = client.get("/api/_/account_metadata", headers=h) j = r.json() assert j["role"] == "user" assert j["logged_in"] == True - h = _register_and_login(client, user_e, mock_ses_client) + +def test_admin_register_and_get_metadata( + client, mock_ses_client, admin_email, valid_redirect_to_url +): + r = client.get("/api/_/account_metadata") + j = r.json() + assert j["logged_in"] == False + h = register_and_login(client, admin_email, mock_ses_client, valid_redirect_to_url) + r = client.get("/api/_/account_metadata", headers=h) + j = r.json() + assert j["role"] == "admin" + assert j["logged_in"] == True + + +def test_user_register_timetravel( + client, mock_ses_client, user_email, valid_redirect_to_url +): + with freeze_time("1990-01-01"): + token = register( + client, + email_address=user_email, + mock_ses_client=mock_ses_client, + valid_redirect_to_url=valid_redirect_to_url, + ) + r = client.get(f"/api/v1/user_login?k={token}") + j = r.json() + assert r.status_code == 200, j + assert len(j["bearer"]) > 100 + assert j["email_address"] == user_email + + h = register_and_login( + client, user_email, mock_ses_client, valid_redirect_to_url + ) + j = client.get("/api/_/account_metadata", headers=h).json() + assert j["logged_in"] == True + assert j["role"] == "user" + + with freeze_time("1995-01-01"): + r = client.get(f"/api/v1/user_login?k={token}") + j = r.json() + assert r.status_code != 200, j diff --git a/ooniapi/services/ooniauth/tests/test_main.py b/ooniapi/services/ooniauth/tests/test_main.py new file mode 100644 index 00000000..58e14244 --- /dev/null +++ b/ooniapi/services/ooniauth/tests/test_main.py @@ -0,0 +1,55 @@ +import pytest + +import httpx +from fastapi.testclient import TestClient +from ooniauth.main import lifespan, app, pkg_version +from ooniauth.common.config import Settings +from ooniauth.common.dependencies import get_settings + + +def test_index(client): + r = client.get("/") + j = r.json() + assert r.status_code == 200 + + +def test_version(client): + r = client.get("/version") + j = r.json() + assert j["version"] == pkg_version + assert len(j["package_name"]) > 1 + + +def test_health_good(client): + r = client.get("/health") + j = r.json() + assert j["status"] == "ok", j + + +def test_health_bad(client_with_bad_settings): + r = client_with_bad_settings.get("/health") + j = r.json() + assert r.status_code != 200 + + +def make_override_get_settings(**kw): + def override_get_settings(): + return Settings(**kw) + + return override_get_settings + + +@pytest.mark.asyncio +async def test_lifecycle(prometheus_password): + app.dependency_overrides[get_settings] = make_override_get_settings( + prometheus_metrics_password=prometheus_password, + ) + + async with lifespan(app) as ls: + client = TestClient(app) + r = client.get("/metrics") + assert r.status_code == 401 + + auth = httpx.BasicAuth(username="prom", password=prometheus_password) + r = client.get("/metrics", auth=auth) + assert r.status_code == 200, r.text