Feat: Integrate direct mobile authentication API & optimize latency#152
Closed
jois-code wants to merge 7 commits into
Closed
Feat: Integrate direct mobile authentication API & optimize latency#152jois-code wants to merge 7 commits into
jois-code wants to merge 7 commits into
Conversation
* feat: migrate to mobile API endpoint for direct async login * refactor: completely remove selectolax/bs4 and CSRF token management * refactor: split authenticate function to reduce cyclomatic complexity (C901) * fix: dynamically skip secret-required integration tests when credentials are not configured * fix: bind application port to environment variable for Render hosting compatibility * style: clean up inline imports and restore Google-style docstrings Simplifies the codebase by using the direct mobile authentication endpoint, removing all HTML parsing and CSRF prefetching overhead. All unit tests now mock the JSON format directly, and functional tests are skipped locally if secrets are not set in the env.
* feat: add _fetch_name_as_in_sslc to retrieve the student's official full name from ISA results * refactor: add _parse_sslc_name helper to resolve Ruff C901 complexity and ANN401 lint errors * feat: implement fallback to raw login display name for new/first-year students who do not have results declared * test: add unit tests covering all branch paths and error conditions of full-name resolution * test: achieve 99% overall test coverage, satisfying the 95% fail-under requirement
achyu-dev
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates pesu-auth from the legacy HTML-scraping login flow to PESU Academy’s mobile authentication endpoint, removing scraper/CSRF-prefetch complexity and updating tests and dependencies accordingly.
Changes:
- Replaced scraper + CSRF-token flow with direct mobile API login and JSON-based response mapping.
- Removed
selectolaxdependency and deleted CSRF/profile-scraping-specific exceptions. - Refactored unit tests and added automatic skipping for secret-dependent functional/integration tests when credentials aren’t configured.
Reviewed changes
Copilot reviewed 8 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
app/pesu.py |
Implements mobile API auth flow, response parsing, and mapping into API response shapes. |
app/app.py |
Simplifies app lifespan/auth endpoint now that CSRF background refresh is removed; adds env-based port default. |
app/exceptions/authentication.py |
Removes scraper-specific exceptions, leaving AuthenticationError. |
tests/unit/test_pesu.py |
Replaces scraper/CSRF tests with mobile API-focused unit tests. |
tests/unit/test_authenticate_unit.py |
Deletes legacy unit tests tied to the old scraper/CSRF implementation. |
tests/unit/test_app_unit.py |
Removes CSRF refresh loop test that no longer applies. |
tests/conftest.py |
Adds env-aware skipping for secret_required tests when credentials are missing. |
pyproject.toml |
Drops selectolax dependency. |
requirements.txt |
Drops selectolax pinned dependency. |
uv.lock |
Removes selectolax from the lockfile. |
.gitignore |
Adds .DS_Store. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+166
to
+168
| prn = data.get("loginId") | ||
| srn = data.get("departmentId") or username | ||
| campus_code = None |
Comment on lines
+214
to
+218
| "section": f"Section {data.get('sectionName')}" if data.get("sectionName") else None, | ||
| "cycle": "NA", | ||
| "department": data.get("branch"), | ||
| "branch": data.get("branch"), | ||
| "instituteName": "PES University", |
Comment on lines
+276
to
285
| try: | ||
| response = await client.post(login_url, data=payload) | ||
| except Exception as e: | ||
| raise AuthenticationError(f"Connection failed: {str(e)}") | ||
|
|
||
| # Check if the response status code is 200 | ||
| if response.status_code != 200: | ||
| raise AuthenticationError( | ||
| f"Authentication failed: Server returned status code {response.status_code}", | ||
| ) |
| "integration": 2, | ||
| } | ||
|
|
||
| has_secrets = os.getenv("TEST_EMAIL") is not None and os.getenv("TEST_PASSWORD") is not None |
| @@ -1,905 +1,239 @@ | |||
| from unittest.mock import AsyncMock, MagicMock, patch | |||
test_authenticate_unit.py was accidentally removed during a LLM-assisted refactor of the test suite. The file has been restored and updated alongside the other unit tests to align with the current mobile-API implementation. - Restore test_authenticate_unit.py with updated authenticate() tests - Refactor test_pesu.py to cover _parse_sslc_name, _fetch_name_as_in_sslc, _map_data, and authenticate() against the new mobile-API flow - Update test_app_unit.py to remove stale _csrf_token_refresh_loop import
…face and FastAPI exception handlers
…rieval from the new mobile API endpoint and expand mappings
achyu-dev
requested changes
Jul 10, 2026
…ion, and migrate integration test file
Comment on lines
+228
to
+229
| prn = data.get("loginId") | ||
| srn = (profile_details.get("loginId") if profile_details else None) or data.get("departmentId") or username |
Comment on lines
+275
to
+277
| kycas_branch = data.get("branch", "").replace("Branch:", "") | ||
| kycas_inst = "PES University (Electronic City)" if campus == "EC" else "PES University (Ring Road Campus)" | ||
|
|
Comment on lines
+351
to
+367
| async with httpx.AsyncClient(follow_redirects=True, timeout=15.0) as client: | ||
| try: | ||
| response = await client.post(login_url, files=files) | ||
| except Exception as e: | ||
| raise AuthenticationError(f"Connection failed: {str(e)}") | ||
|
|
||
| # Check if the response status code is 200 | ||
| if response.status_code != 200: | ||
| raise AuthenticationError( | ||
| f"Authentication failed: Server returned status code {response.status_code}", | ||
| ) | ||
|
|
||
| if know_your_class_and_section: | ||
| logging.info( | ||
| f'"Know Your Class and Section" data requested for user={username}. ' | ||
| 'Fetching "Know Your Class and Section" data...', | ||
| ) | ||
| # Fetch the class and section information | ||
| result["knowYourClassAndSection"] = await self.get_know_your_class_and_section( | ||
| client, | ||
| csrf_token, | ||
| username, | ||
| ) | ||
| # Filter the fields if field filtering is enabled | ||
| if field_filtering: | ||
| result["knowYourClassAndSection"] = { | ||
| key: value for key, value in result["knowYourClassAndSection"].items() if key in fields | ||
| } | ||
| logging.info( | ||
| f'Field filtering enabled. Filtered "Know Your Class and Section" data for user={username}: ' | ||
| f"{result['knowYourClassAndSection']}", | ||
| # Parse the response JSON | ||
| try: | ||
| data = response.json() | ||
| except Exception: | ||
| raise AuthenticationError("Authentication failed: Invalid JSON response from server") |
Comment on lines
+341
to
348
| # Prepare the payload for mobile login API | ||
| login_url = "https://www.pesuacademy.com/MAcademy/mobile/mobilelogin/auth" | ||
| files = { | ||
| "userName": (None, username), | ||
| "password": (None, password), | ||
| "j_appId": (None, "YES"), | ||
| "instId": (None, "1,6,7,14"), | ||
| } |
Comment on lines
+61
to
+62
|
|
||
| asyncio.run(test_httpx()) |
Comment on lines
6
to
11
| class AuthenticationError(PESUAcademyError): | ||
| """Raised when authentication with PESU Academy fails.""" | ||
|
|
||
| def __init__(self, message: str = "Invalid username or password, or user does not exist.") -> None: | ||
| """Initialize the AuthenticationError with a custom message.""" | ||
| super().__init__(message, status_code=401) |
| ) | ||
| from app.exceptions.authentication import AuthenticationError | ||
|
|
||
| # used to to generate list of default field names |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Description
This PR refactors the authentication mechanism of
pesu-authby migrating from the legacy HTML web scraper to the direct mobile login endpoint.selectolax/bs4) and complex CSRF token prefetching loops..env.🧱 Type of Change
namefield now returns display name instead of full name with initials)requirements.txt,pyproject.toml🧪 How Has This Been Tested?
tests/unit/)tests/functional/)tests/integration/)✅ Checklist
scripts/run_tests.py)pre-commit run --all-files)🛠️ Affected API Behaviour
app/app.py– Modified/authenticateroute logicapp/pesu.py– Updated scraping or authentication handling🧩 Models
app/models/profile.py– Profile extraction logic🐳 DevOps & Config
pyproject.toml/requirements.txt– Dependency version changes📸 Screenshots / API Demos (if applicable)
Latency comparison (Current Live Prod in US vs. Singapore Deployment):
200 OK 5.68s200 OK 3.18s200 OK < 1.00s🧠 Additional Notes (if applicable)
"ACHYUTH") instead of the full name with initials ("ACHYUTH JOIS M"). Given the massive 80% reduction in response time, this is a minor compromise for a much faster user experience.