From 635cfd31f1c2aede5485dfa6870424ac70290ba5 Mon Sep 17 00:00:00 2001 From: F-G Fernandez <26927750+frgfm@users.noreply.github.com> Date: Fri, 27 Oct 2023 13:57:08 +0200 Subject: [PATCH 1/9] refactor: Refactor engine creation --- src/app/db.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/app/db.py b/src/app/db.py index cfc0c7f..9a23218 100644 --- a/src/app/db.py +++ b/src/app/db.py @@ -3,10 +3,9 @@ # All rights reserved. # Copying and/or distributing is strictly prohibited without the express permission of its copyright owner. -from sqlalchemy.ext.asyncio import create_async_engine from sqlalchemy.orm import sessionmaker -from sqlmodel import SQLModel, select -from sqlmodel.ext.asyncio.session import AsyncSession +from sqlmodel import SQLModel, create_engine, select +from sqlmodel.ext.asyncio.session import AsyncEngine, AsyncSession from app.core.config import settings from app.core.security import hash_password @@ -15,7 +14,7 @@ __all__ = ["get_session", "init_db"] -engine = create_async_engine(settings.POSTGRES_URL, echo=settings.DEBUG) +engine = AsyncEngine(create_engine(settings.POSTGRES_URL, echo=settings.DEBUG)) async def get_session() -> AsyncSession: # type: ignore[misc] From 429882003a0c43aaf5632d518a8bfa712977ac26 Mon Sep 17 00:00:00 2001 From: F-G Fernandez <26927750+frgfm@users.noreply.github.com> Date: Fri, 27 Oct 2023 16:24:51 +0200 Subject: [PATCH 2/9] chore: Updates pytest plugins --- poetry.lock | 64 +++++++++++++++++++++++++++++++++++++++++++++++++- pyproject.toml | 2 ++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/poetry.lock b/poetry.lock index bafb2d3..d4d316d 100644 --- a/poetry.lock +++ b/poetry.lock @@ -568,6 +568,50 @@ files = [ {file = "h11-0.14.0.tar.gz", hash = "sha256:8f19fbbe99e72420ff35c00b27a34cb9937e902a8b810e2c88300c6f0a3b699d"}, ] +[[package]] +name = "httpcore" +version = "0.18.0" +description = "A minimal low-level HTTP client." +optional = false +python-versions = ">=3.8" +files = [ + {file = "httpcore-0.18.0-py3-none-any.whl", hash = "sha256:adc5398ee0a476567bf87467063ee63584a8bce86078bf748e48754f60202ced"}, + {file = "httpcore-0.18.0.tar.gz", hash = "sha256:13b5e5cd1dca1a6636a6aaea212b19f4f85cd88c366a2b82304181b769aab3c9"}, +] + +[package.dependencies] +anyio = ">=3.0,<5.0" +certifi = "*" +h11 = ">=0.13,<0.15" +sniffio = "==1.*" + +[package.extras] +http2 = ["h2 (>=3,<5)"] +socks = ["socksio (==1.*)"] + +[[package]] +name = "httpx" +version = "0.25.0" +description = "The next generation HTTP client." +optional = false +python-versions = ">=3.8" +files = [ + {file = "httpx-0.25.0-py3-none-any.whl", hash = "sha256:181ea7f8ba3a82578be86ef4171554dd45fec26a02556a744db029a0a27b7100"}, + {file = "httpx-0.25.0.tar.gz", hash = "sha256:47ecda285389cb32bb2691cc6e069e3ab0205956f681c5b2ad2325719751d875"}, +] + +[package.dependencies] +certifi = "*" +httpcore = ">=0.18.0,<0.19.0" +idna = "*" +sniffio = "*" + +[package.extras] +brotli = ["brotli", "brotlicffi"] +cli = ["click (==8.*)", "pygments (==2.*)", "rich (>=10,<14)"] +http2 = ["h2 (>=3,<5)"] +socks = ["socksio (==1.*)"] + [[package]] name = "identify" version = "2.5.30" @@ -1011,6 +1055,24 @@ tomli = {version = ">=1.0.0", markers = "python_version < \"3.11\""} [package.extras] testing = ["argcomplete", "attrs (>=19.2.0)", "hypothesis (>=3.56)", "mock", "nose", "pygments (>=2.7.2)", "requests", "setuptools", "xmlschema"] +[[package]] +name = "pytest-asyncio" +version = "0.21.1" +description = "Pytest support for asyncio" +optional = false +python-versions = ">=3.7" +files = [ + {file = "pytest-asyncio-0.21.1.tar.gz", hash = "sha256:40a7eae6dded22c7b604986855ea48400ab15b069ae38116e8c01238e9eeb64d"}, + {file = "pytest_asyncio-0.21.1-py3-none-any.whl", hash = "sha256:8666c1c8ac02631d7c51ba282e0c69a8a452b211ffedf2599099845da5c5c37b"}, +] + +[package.dependencies] +pytest = ">=7.0.0" + +[package.extras] +docs = ["sphinx (>=5.3)", "sphinx-rtd-theme (>=1.0)"] +testing = ["coverage (>=6.2)", "flaky (>=3.5.0)", "hypothesis (>=5.7.1)", "mypy (>=0.931)", "pytest-trio (>=0.7.0)"] + [[package]] name = "pytest-cov" version = "4.1.0" @@ -1585,4 +1647,4 @@ test = ["covdefaults (>=2.3)", "coverage (>=7.2.7)", "coverage-enable-subprocess [metadata] lock-version = "2.0" python-versions = "^3.9" -content-hash = "c7111900f3e914ca65e93d477b1ef7350c2576366c4626a98f53ce0bfa45ad11" +content-hash = "bd50a403856d66f7d61b1017cfafa347585c39b55e43697d70c32bda987f4975" diff --git a/pyproject.toml b/pyproject.toml index 369cc4c..a2c1a4b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,6 +39,8 @@ pre-commit = { version = "^2.17.0", optional = true } [tool.poetry.group.dev.dependencies] pytest = ">=5.3.2,<8.0.0" +pytest-asyncio = ">=0.17.0,<1.0.0" +httpx = ">=0.23.0" pytest-cov = ">=3.0.0,<5.0.0" pytest-pretty = "^1.0.0" From 67b854eb6090d2800d5942efbd1ee99990b3ac61 Mon Sep 17 00:00:00 2001 From: F-G Fernandez <26927750+frgfm@users.noreply.github.com> Date: Fri, 27 Oct 2023 16:25:57 +0200 Subject: [PATCH 3/9] test: Adds tests for login --- src/tests/conftest.py | 42 +++++++++++++++++++ src/tests/endpoints/test_login.py | 69 +++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+) create mode 100644 src/tests/conftest.py create mode 100644 src/tests/endpoints/test_login.py diff --git a/src/tests/conftest.py b/src/tests/conftest.py new file mode 100644 index 0000000..1fcb117 --- /dev/null +++ b/src/tests/conftest.py @@ -0,0 +1,42 @@ +import asyncio +from typing import Generator + +import pytest +import pytest_asyncio +from httpx import AsyncClient +from sqlalchemy.orm import sessionmaker +from sqlmodel import SQLModel +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.core.config import settings +from app.db import engine +from app.main import app + + +@pytest.fixture(scope="session") +def event_loop(request) -> Generator: + loop = asyncio.get_event_loop_policy().new_event_loop() + yield loop + loop.close() + + +@pytest_asyncio.fixture +async def async_client(): + async with AsyncClient(app=app, base_url=f"http://{settings.API_V1_STR}") as client: + yield client + + +@pytest_asyncio.fixture(scope="function") +async def async_session() -> AsyncSession: + session = sessionmaker(engine, class_=AsyncSession, expire_on_commit=False) + + async with session() as s: + async with engine.begin() as conn: + await conn.run_sync(SQLModel.metadata.create_all) + + yield s + + async with engine.begin() as conn: + await conn.run_sync(SQLModel.metadata.drop_all) + + await engine.dispose() diff --git a/src/tests/endpoints/test_login.py b/src/tests/endpoints/test_login.py new file mode 100644 index 0000000..2eafdf6 --- /dev/null +++ b/src/tests/endpoints/test_login.py @@ -0,0 +1,69 @@ +from typing import Any, Dict, Union +from urllib.parse import parse_qs, urlparse + +import pytest +from httpx import AsyncClient +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.models import User + +USER_TABLE = [ + {"id": 1, "login": "first_login", "hashed_password": "hashed_first_pwd", "scope": "user"}, + {"id": 2, "login": "second_login", "hashed_password": "hashed_second_pwd", "scope": "user"}, +] + + +@pytest.mark.parametrize( + ("payload", "status_code", "status_detail"), + [ + ({"username": "foo"}, 422, None), + ({"github_token": "foo"}, 401, None), + ], +) +@pytest.mark.asyncio() +async def test_login_with_github_token( + async_client: AsyncClient, + async_session: AsyncSession, + payload: Dict[str, Any], + status_code: int, + status_detail: Union[str, None], +): + for entry in USER_TABLE: + async_session.add(User(**entry)) + await async_session.commit() + + response = await async_client.post("/login/token", json=payload) + assert response.status_code == status_code + if isinstance(status_detail, str): + assert response.json()["detail"] == status_detail + + +@pytest.mark.parametrize( + ("scope", "redirect_uri", "status_code"), + [ + ("read:user%20user:email%20repo", "https://app.quack-ai.com", 307), + ], +) +@pytest.mark.asyncio() +async def test_authorize_github( + async_client: AsyncClient, + async_session: AsyncSession, + scope: Any, + redirect_uri: Any, + status_code: int, +): + for entry in USER_TABLE: + async_session.add(User(**entry)) + await async_session.commit() + + response = await async_client.get("/login/authorize", params={"scope": scope, "redirect_uri": redirect_uri}) + assert response.status_code == status_code + for key, _, val in response.headers._list: + if key == "location": + u = urlparse(val) + assert u.scheme == "https" + assert u.netloc == "github.com/login/oauth/authorize" + q = parse_qs(u.query) + assert q.keys() == {"scope", "client_id", "redirect_uri"} + assert q["scope"][0] == scope + assert q["redirect_uri"][0] == redirect_uri From e71ccd0607083056db5b3e8fbf3d29599e2e3999 Mon Sep 17 00:00:00 2001 From: F-G Fernandez <26927750+frgfm@users.noreply.github.com> Date: Fri, 27 Oct 2023 16:32:44 +0200 Subject: [PATCH 4/9] test: Refactored tests --- src/tests/endpoints/test_login.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/tests/endpoints/test_login.py b/src/tests/endpoints/test_login.py index 2eafdf6..e2bc5de 100644 --- a/src/tests/endpoints/test_login.py +++ b/src/tests/endpoints/test_login.py @@ -2,6 +2,7 @@ from urllib.parse import parse_qs, urlparse import pytest +import pytest_asyncio from httpx import AsyncClient from sqlmodel.ext.asyncio.session import AsyncSession @@ -13,6 +14,14 @@ ] +@pytest_asyncio.fixture +async def session(async_session: AsyncSession): + for entry in USER_TABLE: + async_session.add(User(**entry)) + await async_session.commit() + yield async_session + + @pytest.mark.parametrize( ("payload", "status_code", "status_detail"), [ @@ -23,15 +32,11 @@ @pytest.mark.asyncio() async def test_login_with_github_token( async_client: AsyncClient, - async_session: AsyncSession, + session: AsyncSession, payload: Dict[str, Any], status_code: int, status_detail: Union[str, None], ): - for entry in USER_TABLE: - async_session.add(User(**entry)) - await async_session.commit() - response = await async_client.post("/login/token", json=payload) assert response.status_code == status_code if isinstance(status_detail, str): @@ -47,15 +52,11 @@ async def test_login_with_github_token( @pytest.mark.asyncio() async def test_authorize_github( async_client: AsyncClient, - async_session: AsyncSession, + session: AsyncSession, scope: Any, redirect_uri: Any, status_code: int, ): - for entry in USER_TABLE: - async_session.add(User(**entry)) - await async_session.commit() - response = await async_client.get("/login/authorize", params={"scope": scope, "redirect_uri": redirect_uri}) assert response.status_code == status_code for key, _, val in response.headers._list: From 3e2e1a28d7e1a4a3a80fa03e6ad6ba0ba4285e18 Mon Sep 17 00:00:00 2001 From: F-G Fernandez <26927750+frgfm@users.noreply.github.com> Date: Fri, 27 Oct 2023 16:33:26 +0200 Subject: [PATCH 5/9] docs: Updates makefile --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 7c08430..0f5a2f3 100644 --- a/Makefile +++ b/Makefile @@ -1,13 +1,13 @@ # this target runs checks on all files quality: + ruff format --check . ruff check . mypy - ruff format --check . # this target runs checks on all files and potentially modifies some of them style: - ruff --fix . ruff format . + ruff --fix . # Pin the dependencies lock: From e5783ef0d7cb1a114c589f79edd8e2aaa69e95e9 Mon Sep 17 00:00:00 2001 From: F-G Fernandez <26927750+frgfm@users.noreply.github.com> Date: Fri, 27 Oct 2023 17:44:52 +0200 Subject: [PATCH 6/9] test: Adds test --- src/tests/conftest.py | 9 +++++ src/tests/endpoints/test_login.py | 59 +++++++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/tests/conftest.py b/src/tests/conftest.py index 1fcb117..4f1edac 100644 --- a/src/tests/conftest.py +++ b/src/tests/conftest.py @@ -40,3 +40,12 @@ async def async_session() -> AsyncSession: await conn.run_sync(SQLModel.metadata.drop_all) await engine.dispose() + + +async def mock_verify_password(plain_password, hashed_password): + return hashed_password == f"hashed_{plain_password}" + + +def pytest_configure(): + # api.security patching + pytest.mock_verify_password = mock_verify_password diff --git a/src/tests/endpoints/test_login.py b/src/tests/endpoints/test_login.py index e2bc5de..fa2c763 100644 --- a/src/tests/endpoints/test_login.py +++ b/src/tests/endpoints/test_login.py @@ -6,6 +6,7 @@ from httpx import AsyncClient from sqlmodel.ext.asyncio.session import AsyncSession +from app.core import security from app.models import User USER_TABLE = [ @@ -14,11 +15,12 @@ ] -@pytest_asyncio.fixture -async def session(async_session: AsyncSession): +@pytest_asyncio.fixture(scope="function") +async def session(async_session: AsyncSession, monkeypatch): for entry in USER_TABLE: async_session.add(User(**entry)) await async_session.commit() + monkeypatch.setattr(security, "verify_password", pytest.mock_verify_password) yield async_session @@ -43,6 +45,59 @@ async def test_login_with_github_token( assert response.json()["detail"] == status_detail +@pytest.mark.parametrize( + ("payload", "status_code", "status_detail"), + [ + ({"username": "foo"}, 422, None), + ({"username": "foo", "password": "bar"}, 401, None), + # ({"username": "first_login", "password": "first_pwd"}, 200, None), + ], +) +@pytest.mark.asyncio() +async def test_login_with_creds( + async_client: AsyncClient, + session: AsyncSession, + payload: Dict[str, Any], + status_code: int, + status_detail: Union[str, None], +): + response = await async_client.post("/login/creds", data=payload) + assert response.status_code == status_code + if isinstance(status_detail, str): + assert response.json()["detail"] == status_detail + if response.status_code // 100 == 2: + response_json = response.json() + assert response_json["token_type"] == "Bearer" # noqa: S105 + assert isinstance(response_json["access_token"], str) + assert len(response_json["access_token"]) == 10 + + +@pytest.mark.parametrize( + ("payload", "status_code", "status_detail", "expected_response"), + [ + ({"code": "foo", "redirect_uri": 0}, 422, None, None), + # Github 422 + ({"code": "foo", "redirect_uri": ""}, 422, None, None), + ({"code": "foo", "redirect_uri": "https://quackai.com"}, 401, None, None), + ], +) +@pytest.mark.asyncio() +async def test_request_github_token_from_code( + async_client: AsyncClient, + session: AsyncSession, + payload: Dict[str, Any], + status_code: int, + status_detail: Union[str, None], + expected_response: Union[Dict[str, Any], None], +): + response = await async_client.post("/login/github", json=payload) + assert response.status_code == status_code + if isinstance(status_detail, str): + assert response.json()["detail"] == status_detail + if isinstance(expected_response, dict): + assert response.json() == expected_response + + @pytest.mark.parametrize( ("scope", "redirect_uri", "status_code"), [ From b1391ac14f2a3b9ebd670a96682f1d44f6b621f1 Mon Sep 17 00:00:00 2001 From: F-G Fernandez <26927750+frgfm@users.noreply.github.com> Date: Fri, 27 Oct 2023 17:46:00 +0200 Subject: [PATCH 7/9] fix: Fixes login --- src/app/api/api_v1/endpoints/login.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/app/api/api_v1/endpoints/login.py b/src/app/api/api_v1/endpoints/login.py index 6afeb95..6385032 100644 --- a/src/app/api/api_v1/endpoints/login.py +++ b/src/app/api/api_v1/endpoints/login.py @@ -49,7 +49,7 @@ async def request_github_token_from_code( headers={"Accept": "application/json"}, timeout=5, ) - if response.status_code != status.HTTP_200_OK: + if response.status_code != status.HTTP_200_OK or isinstance(response.json().get("error"), str): raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid authorization code.") return GHToken(**response.json()) @@ -66,12 +66,13 @@ async def login_with_creds( By default, the token expires after 1 hour. """ # Verify credentials - entry = await users.get_by_login(form_data.username) - if entry is None or not await verify_password(form_data.password, entry.hashed_password): + user = await users.get_by_login(form_data.username) + if user is None or not await verify_password(form_data.password, user.hashed_password): raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid credentials.") # create access token using user user_id/user_scopes - token_data = {"sub": str(entry.id), "scopes": entry.scope.split()} + token_data = {"sub": str(user.id), "scopes": user.scope.split()} token = await create_access_token(token_data, settings.ACCESS_TOKEN_UNLIMITED_MINUTES) + analytics_client.capture(user.id, event="user-login", properties={"login": user.login}) return Token(access_token=token, token_type="bearer") # nosec B106 # noqa S106 @@ -122,5 +123,6 @@ async def login_with_github_token( # create access token using user user_id/user_scopes token_data = {"sub": str(user.id), "scopes": user.scope.split()} token = await create_access_token(token_data, settings.ACCESS_TOKEN_UNLIMITED_MINUTES) + analytics_client.capture(user.id, event="user-login", properties={"login": user.login}) return Token(access_token=token, token_type="bearer") # nosec B106 # noqa S106 From f1c20e099d15296c239a8f1b20a282df071d6526 Mon Sep 17 00:00:00 2001 From: F-G Fernandez <26927750+frgfm@users.noreply.github.com> Date: Fri, 27 Oct 2023 17:51:22 +0200 Subject: [PATCH 8/9] style: Fixes typing --- src/app/db.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/db.py b/src/app/db.py index 9a23218..37079d6 100644 --- a/src/app/db.py +++ b/src/app/db.py @@ -3,9 +3,10 @@ # All rights reserved. # Copying and/or distributing is strictly prohibited without the express permission of its copyright owner. +from sqlalchemy.ext.asyncio.engine import AsyncEngine from sqlalchemy.orm import sessionmaker from sqlmodel import SQLModel, create_engine, select -from sqlmodel.ext.asyncio.session import AsyncEngine, AsyncSession +from sqlmodel.ext.asyncio.session import AsyncSession from app.core.config import settings from app.core.security import hash_password From f63c82b5e93d4f6baaa1a088f649c9035ea70dbe Mon Sep 17 00:00:00 2001 From: F-G Fernandez <26927750+frgfm@users.noreply.github.com> Date: Fri, 27 Oct 2023 17:52:29 +0200 Subject: [PATCH 9/9] feat: Adds telemetry in repo fetching --- src/app/api/api_v1/endpoints/repos.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/api/api_v1/endpoints/repos.py b/src/app/api/api_v1/endpoints/repos.py index 3e4224c..883bf3c 100644 --- a/src/app/api/api_v1/endpoints/repos.py +++ b/src/app/api/api_v1/endpoints/repos.py @@ -56,6 +56,7 @@ async def fetch_repos( user=Security(get_current_user, scopes=[UserScope.USER, UserScope.ADMIN]), ) -> List[Repository]: entries = await repos.fetch_all() if user.scope == UserScope.ADMIN else await repos.fetch_all(("owner_id", user.id)) + analytics_client.capture(user.id, event="repo-fetch") return [elt for elt in entries]