diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index 2847410..c0152ec 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -36,3 +36,23 @@ jobs: - name: Run unit tests run: poetry run pytest + + pyright: + runs-on: ubuntu-latest + steps: + - name: Check out repository + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Install Poetry + run: pip install poetry + + - name: Install dependencies + run: poetry install + + - name: Run Pyright + run: poetry run pyright diff --git a/campus_python/__init__.py b/campus_python/__init__.py index 65168cb..a545d81 100644 --- a/campus_python/__init__.py +++ b/campus_python/__init__.py @@ -163,7 +163,7 @@ def audit(self) -> AuditRoot: mode="device", ) env.require("AUDIT_API_KEY") - client.set_bearer_authorization(env.get("AUDIT_API_KEY")) + client.set_bearer_authorization(env.get("AUDIT_API_KEY") or "") self._audit = AuditRoot(json_client=client) return self._audit @@ -190,7 +190,12 @@ def _get_token_from_session( """Get the token from flask session.""" # flask session stores a session id login_session = self.auth.logins.from_session() - creds_resource = self.auth.credentials["campus"][login_session.user_id] + user_id = login_session.user_id + if user_id is None: + raise errors.AuthenticationError( + error_description="Login session has no user id." + ) + creds_resource = self.auth.credentials["campus"][user_id] user_creds = creds_resource.get() if ( force_refresh @@ -200,9 +205,7 @@ def _get_token_from_session( grant_type="refresh_token", refresh_token=user_creds.token.refresh_token ) - self.auth.credentials["campus"][login_session.user_id].update( - token=token - ) + self.auth.credentials["campus"][user_id].update(token=token) # Return the refreshed token — user_creds still holds the # pre-refresh (now rotated) access token. return token @@ -307,8 +310,11 @@ def refresh_bearer() -> str | None: return refreshed.access_token for client in (self.api.client, self.auth.client): - client.set_unauthorized_hook(refresh_bearer) - hooked_clients.append(client) + # The 401 hook is an optional JsonClient capability + # (see json_client.interface) — feature-detect it. + if isinstance(client, CampusRequest): + client.set_unauthorized_hook(refresh_bearer) + hooked_clients.append(client) try: yield self diff --git a/campus_python/api/v1/assignments.py b/campus_python/api/v1/assignments.py index fcd5433..7634314 100644 --- a/campus_python/api/v1/assignments.py +++ b/campus_python/api/v1/assignments.py @@ -45,7 +45,7 @@ def new( questions: "list[dict] | None" = None, classroom_links: "list[dict] | None" = None, ) -> campus.model.Assignment: - payload = {"title": title} + payload: "dict[str, object]" = {"title": title} if description is not None: payload["description"] = description if questions is not None: diff --git a/campus_python/audit/ratelimit.py b/campus_python/audit/ratelimit.py index 9b8e2c2..1ecc5df 100644 --- a/campus_python/audit/ratelimit.py +++ b/campus_python/audit/ratelimit.py @@ -200,10 +200,12 @@ def _bucket_from_body(body: typing.Any) -> str | None: def _retry_after_seconds(headers: typing.Mapping[str, str] | None) -> float: """Parse Retry-After from response headers, defaulting to the floor.""" if headers: - try: - return float(headers.get("Retry-After")) - except (TypeError, ValueError): - pass + retry_after = headers.get("Retry-After") + if retry_after is not None: + try: + return float(retry_after) + except ValueError: + pass return BACKOFF_FLOOR_SECONDS diff --git a/campus_python/auth/v1/__init__.py b/campus_python/auth/v1/__init__.py index c8a3cae..e7895f1 100644 --- a/campus_python/auth/v1/__init__.py +++ b/campus_python/auth/v1/__init__.py @@ -332,6 +332,10 @@ def get_token(self) -> campus.model.OAuthToken: ) login_session = self.logins.from_session() user_id = login_session.user_id + if user_id is None: + raise errors.AuthenticationError( + error_description="Login session has no user id." + ) # Retrieve credentials for the user credentials = self.credentials["campus"][user_id].get() @@ -432,15 +436,16 @@ def push_context(self) -> None: try: login_session = self.logins.from_session() # Embedded login user (#879) with the same fallback. - user = ( - getattr(login_session, "user", None) - or self.users[login_session.user_id].get() - ) + login_user_id = login_session.user_id + user = getattr(login_session, "user", None) + if user is None and login_user_id is not None: + user = self.users[login_user_id].get() flask.g.user = user flask.g.device = login_session.device_id - self._user_cache_put( - "login", login_id, user, login_session.device_id - ) + if user is not None: + self._user_cache_put( + "login", login_id, user, login_session.device_id + ) except errors.NotFoundError: # Login session no longer exists on server self._user_cache_pop("login", login_id) diff --git a/campus_python/auth/v1/oauth.py b/campus_python/auth/v1/oauth.py index e8be71c..8bb5413 100644 --- a/campus_python/auth/v1/oauth.py +++ b/campus_python/auth/v1/oauth.py @@ -169,7 +169,7 @@ def poll_for_token( raise errors.AuthenticationError( status_code=400, error_description=( - descriptions.get(oauth_error) + descriptions.get(oauth_error or "") or message or "Unknown error" ), diff --git a/campus_python/json_client/__init__.py b/campus_python/json_client/__init__.py index 46d4ce5..c7904c8 100644 --- a/campus_python/json_client/__init__.py +++ b/campus_python/json_client/__init__.py @@ -15,14 +15,7 @@ import requests from campus.common import env - -try: - from campus.webauth.models import HttpHeader -except ImportError: - # campus-suite predating campus#857 keeps header models in - # campus.model; this fallback lets this client work against both - # the old and the new campus-suite until locks are re-resolved. - from campus.model import HttpHeader +from campus.webauth.models import HttpHeader from .. import errors from .interface import JsonClient, JsonDict, JsonResponse diff --git a/poetry.lock b/poetry.lock index bc479ed..2fb261d 100644 --- a/poetry.lock +++ b/poetry.lock @@ -568,6 +568,21 @@ files = [ {file = "markupsafe-3.0.3.tar.gz", hash = "sha256:722695808f4b6457b320fdc131280796bdceb04ab50fe1795cd540799ebe1698"}, ] +[[package]] +name = "nodeenv" +version = "1.11.0" +description = "Node.js virtual environment builder" +optional = false +python-versions = "!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*,!=3.4.*,!=3.5.*,!=3.6.*,>=2.7" +groups = ["dev"] +files = [ + {file = "nodeenv-1.11.0-py2.py3-none-any.whl", hash = "sha256:edaa16e6c14d7cf395d75d4bbd5a26390f4dc06501a33b4e76282b02cc688a25"}, + {file = "nodeenv-1.11.0.tar.gz", hash = "sha256:3ce8fe5b71d16e8af7039ca65257354100bc772965d6bc549070649e53b1b146"}, +] + +[package.extras] +certifi = ["certifi"] + [[package]] name = "packaging" version = "26.2" @@ -782,6 +797,27 @@ snappy = ["python-snappy (>=0.6.0)"] test = ["importlib-metadata (>=7.0) ; python_version < \"3.13\"", "pytest (>=8.2)", "pytest-asyncio (>=0.24.0)"] zstd = ["backports-zstd (>=1.0.0) ; python_version < \"3.14\""] +[[package]] +name = "pyright" +version = "1.1.414" +description = "Command line wrapper for pyright" +optional = false +python-versions = ">=3.7" +groups = ["dev"] +files = [ + {file = "pyright-1.1.414-py3-none-any.whl", hash = "sha256:2a6b4b3298c9eec174c5ed83bd338de6eee82df2992f3e1930e6199d381be36f"}, + {file = "pyright-1.1.414.tar.gz", hash = "sha256:523c0a97c60da6333234955c277730c9cf4f5bd6d5399e7b7d2b0fc5d3599524"}, +] + +[package.dependencies] +nodeenv = ">=1.6.0" +typing-extensions = ">=4.1" + +[package.extras] +all = ["nodejs-wheel-binaries", "twine (>=3.4.1)"] +dev = ["twine (>=3.4.1)"] +nodejs = ["nodejs-wheel-binaries"] + [[package]] name = "pytest" version = "8.4.2" @@ -871,6 +907,18 @@ files = [ {file = "six-1.17.0.tar.gz", hash = "sha256:ff70335d468e7eb6ec65b95b99d3a2836546063f63acc5171de367e834932a81"}, ] +[[package]] +name = "typing-extensions" +version = "4.16.0" +description = "Backported and Experimental Type Hints for Python 3.9+" +optional = false +python-versions = ">=3.9" +groups = ["dev"] +files = [ + {file = "typing_extensions-4.16.0-py3-none-any.whl", hash = "sha256:481caa481374e813c1b176ada14e97f1f67a4539ce9cfeb3f350d78d6370c2e8"}, + {file = "typing_extensions-4.16.0.tar.gz", hash = "sha256:dc983d19a509c94dba722ee6abd33940f7c05a89e243c47e907eb4db6f1a43e5"}, +] + [[package]] name = "urllib3" version = "2.7.0" @@ -910,4 +958,4 @@ watchdog = ["watchdog (>=2.3)"] [metadata] lock-version = "2.1" python-versions = ">=3.11,<3.15" -content-hash = "96efa4aa931d87d618c0f34681daf6ffba10a359baae98aa4de94666c9d132cd" +content-hash = "bae667cec288e6b7229ec72df81c76f9e788083bf0c1427a263dc6281528315c" diff --git a/pyproject.toml b/pyproject.toml index 5f6a992..8697597 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,6 +15,7 @@ campus-suite = {git = "https://github.com/nyjc-computing/campus.git", branch = " [tool.poetry.group.dev.dependencies] pytest = "^8.4.2" +pyright = "^1.1.400" [[tool.poetry.packages]] include = "campus_python" diff --git a/tests/unit/test_audit.py b/tests/unit/test_audit.py index ad76110..413795d 100644 --- a/tests/unit/test_audit.py +++ b/tests/unit/test_audit.py @@ -11,10 +11,12 @@ import os import unittest +from typing import cast from unittest.mock import Mock, patch from campus_python import Campus from campus_python.audit.v1 import AuditRoot +from campus_python.json_client import CampusRequest API_KEY_RESOURCE = { "id": "akm123", @@ -187,7 +189,7 @@ def test_audit_root_uses_audit_url_and_key(self): audit = campus.audit self.assertEqual(audit.base_url, "https://audit.example.test") self.assertEqual( - audit.client._session.headers["Authorization"], + cast(CampusRequest, audit.client)._session.headers["Authorization"], "Bearer audit_v1_testkey", ) self.assertEqual(audit.url_prefix, "/audit/v1") diff --git a/tests/unit/test_campus_request.py b/tests/unit/test_campus_request.py index 35df32e..b96d02e 100644 --- a/tests/unit/test_campus_request.py +++ b/tests/unit/test_campus_request.py @@ -65,6 +65,7 @@ def test_server_mode_with_credentials(self): # Verify Basic auth header was set auth_header = client._session.headers.get('Authorization') self.assertIsNotNone(auth_header) + assert auth_header is not None self.assertTrue(auth_header.startswith('Basic ')) def test_default_mode_is_server(self): @@ -101,6 +102,7 @@ def test_bearer_authorization_overrides_basic(self): # Verify Basic auth was set initially auth_header = client._session.headers.get('Authorization') + assert auth_header is not None self.assertTrue(auth_header.startswith('Basic ')) # Set Bearer authorization (should override) diff --git a/tests/unit/test_embedded_session_user.py b/tests/unit/test_embedded_session_user.py index ca0127b..026bb50 100644 --- a/tests/unit/test_embedded_session_user.py +++ b/tests/unit/test_embedded_session_user.py @@ -10,6 +10,7 @@ import os import unittest from types import SimpleNamespace +from typing import Any from unittest.mock import MagicMock, Mock, patch import campus.model @@ -68,7 +69,7 @@ class TestFinalizeUsesEmbed(unittest.TestCase): """finalize() answers step 4 from the embed when present.""" def setUp(self): - self.auth, _ = make_auth() + self.auth, self.client_mock = make_auth() def run_finalize(self, finalize_return: tuple) -> None: sessions = MagicMock() @@ -83,11 +84,12 @@ def run_finalize(self, finalize_return: tuple) -> None: users = MagicMock() users["user-1"].get.return_value = EMBEDDED_USER self.auth._users = users + self.users_mock = users logins = MagicMock() self.auth._logins = logins token_response = Mock() token_response.json.return_value = TOKEN_RESOURCE - self.auth.client.post.return_value = token_response + self.client_mock.post.return_value = token_response app = flask.Flask(__name__) app.secret_key = "test-secret" @@ -101,12 +103,12 @@ def run_finalize(self, finalize_return: tuple) -> None: def test_embedded_user_skips_users_route(self): """The embed answers the user-exists check without users.get.""" self.run_finalize(("https://app.example.org/landing", EMBEDDED_USER)) - self.assertFalse(self.auth._users["user-1"].get.called) + self.assertFalse(self.users_mock["user-1"].get.called) def test_no_embed_falls_back_to_users_route(self): """Pre-#879 services return no embed; users.get stays the check.""" self.run_finalize(("https://app.example.org/landing", None)) - self.assertTrue(self.auth._users["user-1"].get.called) + self.assertTrue(self.users_mock["user-1"].get.called) class TestPushContextUsesEmbed(unittest.TestCase): @@ -115,7 +117,7 @@ class TestPushContextUsesEmbed(unittest.TestCase): def setUp(self): self.auth, _ = make_auth() - def push_session(self, session_obj) -> object: + def push_session(self, session_obj) -> Any: sessions = MagicMock() sessions._session_key = "campus_session_id" sessions.has_session.return_value = True @@ -124,12 +126,13 @@ def push_session(self, session_obj) -> object: users = MagicMock() users["user-1"].get.return_value = EMBEDDED_USER self.auth._users = users + self.users_mock = users with session_context(): flask.session["campus_session_id"] = SESSION_ID self.auth.push_context() return flask.g.user - def push_login(self, login_obj) -> object: + def push_login(self, login_obj) -> Any: logins = MagicMock() logins._session_key = "logins_login_id" logins.has_session.return_value = True @@ -138,6 +141,7 @@ def push_login(self, login_obj) -> object: users = MagicMock() users["user-1"].get.return_value = EMBEDDED_USER self.auth._users = users + self.users_mock = users with session_context(): flask.session["logins_login_id"] = "login-1" self.auth.push_context() @@ -147,13 +151,13 @@ def test_session_embed_skips_users_route(self): session = SimpleNamespace(user_id="user-1", user=EMBEDDED_USER) user = self.push_session(session) self.assertEqual(user.email, USER_RESOURCE["email"]) - self.assertFalse(self.auth._users["user-1"].get.called) + self.assertFalse(self.users_mock["user-1"].get.called) def test_session_without_embed_falls_back(self): session = SimpleNamespace(user_id="user-1", user=None) user = self.push_session(session) self.assertEqual(user.email, USER_RESOURCE["email"]) - self.assertTrue(self.auth._users["user-1"].get.called) + self.assertTrue(self.users_mock["user-1"].get.called) def test_model_without_user_field_falls_back(self): """An installed campus-suite predating the field still resolves.""" @@ -166,7 +170,7 @@ def test_model_without_user_field_falls_back(self): ) user = self.push_session(session) self.assertEqual(user.email, USER_RESOURCE["email"]) - self.assertTrue(self.auth._users["user-1"].get.called) + self.assertTrue(self.users_mock["user-1"].get.called) def test_login_embed_skips_users_route(self): login = SimpleNamespace( @@ -174,13 +178,13 @@ def test_login_embed_skips_users_route(self): ) user = self.push_login(login) self.assertEqual(user.email, USER_RESOURCE["email"]) - self.assertFalse(self.auth._users["user-1"].get.called) + self.assertFalse(self.users_mock["user-1"].get.called) def test_login_without_embed_falls_back(self): login = SimpleNamespace(user_id="user-1", user=None, device_id=None) user = self.push_login(login) self.assertEqual(user.email, USER_RESOURCE["email"]) - self.assertTrue(self.auth._users["user-1"].get.called) + self.assertTrue(self.users_mock["user-1"].get.called) if __name__ == "__main__": diff --git a/tests/unit/test_errors.py b/tests/unit/test_errors.py index a1ca58d..7a99f6a 100644 --- a/tests/unit/test_errors.py +++ b/tests/unit/test_errors.py @@ -14,7 +14,7 @@ import unittest -from errors import ( +from errors import ( # pyright: ignore[reportMissingImports] APIError, BadRequestError, FieldError, diff --git a/tests/unit/test_finalize_device_id.py b/tests/unit/test_finalize_device_id.py index 1a1827f..b749c08 100644 --- a/tests/unit/test_finalize_device_id.py +++ b/tests/unit/test_finalize_device_id.py @@ -9,6 +9,7 @@ import os import re import unittest +from typing import cast from unittest.mock import MagicMock, Mock, patch import campus.model @@ -63,7 +64,7 @@ def run_finalize(auth: AuthRoot, device_id: str | None) -> dict: # _exchange_code_for_token POSTs to /auth/v1/token token_response = Mock() token_response.json.return_value = TOKEN_RESOURCE - auth.client.post.return_value = token_response + cast(MagicMock, auth.client.post).return_value = token_response app = flask.Flask(__name__) app.secret_key = "test-secret" diff --git a/tests/unit/test_oauth_device_flow.py b/tests/unit/test_oauth_device_flow.py index 358ddc6..ea61397 100644 --- a/tests/unit/test_oauth_device_flow.py +++ b/tests/unit/test_oauth_device_flow.py @@ -297,7 +297,7 @@ def test_timeout_raises_authentication_error(self): with self.assertRaises(errors.AuthenticationError) as ctx: self.wait(max_attempts=5) self.assertIsNone(ctx.exception.oauth_error) - self.assertIn("timed out", ctx.exception.error_description) + self.assertIn("timed out", ctx.exception.error_description or "") def test_default_interval_when_server_omits_it(self): self.client.post.side_effect = [self.pending(), self.token_response()] diff --git a/tests/unit/test_oauth_token_contract.py b/tests/unit/test_oauth_token_contract.py index 87fb440..5a44b06 100644 --- a/tests/unit/test_oauth_token_contract.py +++ b/tests/unit/test_oauth_token_contract.py @@ -261,7 +261,7 @@ class TestExpirySecondsAliasRemoved(unittest.TestCase): def test_constructor_rejects_expiry_seconds_kwarg(self): with self.assertRaises(TypeError): - campus.model.OAuthToken(id="tok-1", expiry_seconds=60) + campus.model.OAuthToken(id="tok-1", expiry_seconds=60) # pyright: ignore[reportCallIssue] def test_payload_with_only_expiry_seconds_fails_validation(self): """Mirrors the server-side validation: a payload whose only diff --git a/tests/unit/test_ratelimit.py b/tests/unit/test_ratelimit.py index 591abb9..eb47f4c 100644 --- a/tests/unit/test_ratelimit.py +++ b/tests/unit/test_ratelimit.py @@ -46,6 +46,7 @@ def test_429_trips_with_retry_after(self): self.br.observe(429, {"Retry-After": "45"}, _body("user=u1")) retry_after = self.br.check("user=u1") self.assertIsNotNone(retry_after) + assert retry_after is not None self.assertGreaterEqual(retry_after, 44) self.assertLessEqual(retry_after, 46) @@ -54,6 +55,7 @@ def test_429_floor_30s(self): self.br.observe(429, None, _body("user=u1")) retry_after = self.br.check("user=u1") self.assertIsNotNone(retry_after) + assert retry_after is not None self.assertGreaterEqual(retry_after, 29) def test_429_without_bucket_trips_wildcard(self): diff --git a/tests/unit/test_request_guards.py b/tests/unit/test_request_guards.py index d392de0..b656176 100644 --- a/tests/unit/test_request_guards.py +++ b/tests/unit/test_request_guards.py @@ -29,7 +29,7 @@ def setUp(self): def test_update_requires_name(self): with self.assertRaises(TypeError): - self.api.circles["cir-1"].update() + self.api.circles["cir-1"].update() # pyright: ignore[reportCallIssue] self.client.patch.assert_not_called() def test_update_sends_name(self): diff --git a/tests/unit/test_sessions.py b/tests/unit/test_sessions.py index 078fabf..5299431 100644 --- a/tests/unit/test_sessions.py +++ b/tests/unit/test_sessions.py @@ -83,6 +83,7 @@ def test_finalize_parses_embedded_user(self): target, user = self.auth.sessions["sess-1"].finalize() self.assertEqual(target, "https://app.example.org/after-login") self.assertIsNotNone(user) + assert user is not None self.assertEqual(user.id, "user-1") self.assertEqual(user.email, "user@example.org") diff --git a/tests/unit/test_trace_propagation.py b/tests/unit/test_trace_propagation.py index 58554c7..81500c0 100644 --- a/tests/unit/test_trace_propagation.py +++ b/tests/unit/test_trace_propagation.py @@ -31,8 +31,8 @@ def _capture_send(client: CampusRequest) -> dict: """ captured: dict = {} - def fake_send(prepared: requests.PreparedRequest, **_kwargs): - captured["headers"] = dict(prepared.headers) + def fake_send(request: requests.PreparedRequest, **_kwargs): + captured["headers"] = dict(request.headers) response = requests.Response() response.status_code = 200 response.headers["Content-Type"] = "application/json" diff --git a/tests/unit/test_unauthorized_refresh.py b/tests/unit/test_unauthorized_refresh.py index 51895ae..12d0584 100644 --- a/tests/unit/test_unauthorized_refresh.py +++ b/tests/unit/test_unauthorized_refresh.py @@ -14,6 +14,7 @@ import json import os import unittest +from typing import cast from unittest import mock from unittest.mock import MagicMock, Mock @@ -51,8 +52,9 @@ def setUp(self): self.client = make_client() def set_responses(self, *responses: requests.Response) -> MagicMock: - self.client._session.request.side_effect = list(responses) - return self.client._session.request + session = cast(MagicMock, self.client._session) + session.request.side_effect = list(responses) + return session.request def test_401_triggers_hook_and_retries(self): request_mock = self.set_responses( @@ -180,7 +182,7 @@ def setUp(self): self.get_token.return_value = token1 def test_401_mid_session_force_refreshes_and_retries(self): - self.api_client._session.request.side_effect = [ + cast(MagicMock, self.api_client._session).request.side_effect = [ raw_response(401, {}), raw_response(200, {"ok": True}), ] token2 = Mock(access_token="tok2") @@ -204,7 +206,7 @@ def test_401_mid_session_force_refreshes_and_retries(self): self.assertTrue(self.get_token.call_args.kwargs["force_refresh"]) def test_refresh_failure_surfaces_original_401(self): - self.api_client._session.request.side_effect = [raw_response(401, {})] + cast(MagicMock, self.api_client._session).request.side_effect = [raw_response(401, {})] self.get_token.side_effect = [ Mock(access_token="tok1"), errors.AuthenticationError(error_description="session gone"), @@ -219,7 +221,7 @@ def test_refresh_failure_surfaces_original_401(self): ) self.assertEqual(resp.status_code, 401) - self.assertEqual(self.api_client._session.request.call_count, 1) + self.assertEqual(cast(MagicMock, self.api_client._session).request.call_count, 1) def test_hooks_cleared_after_session(self): with self.campus.with_user_session(): @@ -230,7 +232,7 @@ def test_hooks_cleared_after_session(self): self.assertIsNone(self.auth_client._unauthorized_hook) def test_refresh_on_401_false_keeps_old_behaviour(self): - self.api_client._session.request.side_effect = [raw_response(401, {})] + cast(MagicMock, self.api_client._session).request.side_effect = [raw_response(401, {})] with self.campus.with_user_session(refresh_on_401=False): self.assertIsNone(self.api_client._unauthorized_hook)