Skip to content

feat!: release 5.0.0; sign in through the PESU Academy mobile API - #241

Merged
aditeyabaral merged 54 commits into
pesu-dev:devfrom
aditeyabaral:feat/mobile-api-migration
Oct 8, 2026
Merged

aditeyabaral merged 54 commits into
pesu-dev:devfrom
aditeyabaral:feat/mobile-api-migration

Conversation

@aditeyabaral

@aditeyabaral aditeyabaral commented Oct 4, 2026 •

Copy link
Copy Markdown
Member

📌 Description

Release 5.0.0. PESUAuth now signs in and fetches profiles through the JSON API that the PESU Academy mobile app uses, instead of scraping the web portal. The profile grows to 17 fields, each taken as PESU sends it from one labelled source. This was started from scratch, as proposed in #233; #152 was used only as a reference for the upstream protocol.

Why. The web flow needed a CSRF token prefetch, a cached client, a 45-minute refresh loop and background prefetch tasks, then scraped the profile page's HTML. Any markup change broke profiles with a 422, the CSRF machinery was behind most of this module's leaks and races, and a profile request took about 4 s.

ℹ️ Fixes / Related Issues
Fixes: #233
Related: #152

How it works now

  • Login: each request opens its own HTTP client and posts the credentials to /MAcademy/mobile/mobilelogin/auth. The password is sent only there, and never stored or logged.
  • Profile (only if requested): a second call to /MAcademy/mobile/dispatcher, authorised with the access token from the login.
  • No state between requests: the client is closed on every path, including cancellation. Nothing is cached or prepared at startup.
  • Narrow parsing: PESU's responses are parsed into models that keep only the fields we return. Photos, blood group, addresses, marks, admission data and parents' details are dropped at parse time and can't be requested. Parse failures log field locations only, never values, and the access token never reaches a log.

The profile: 17 fields

Every field is copied from PESU's profile response as sent, apart from the two marked derived. Fields marked new were not returned by the web flow.

Field Source Example Notes
name STUDENT_INFO.NameAsInSSLC JOHN DOE
prn STUDENT_INFO.LoginId PES1202000001 No format check
srn STUDENT_INFO.SRN PES1UG20CS001 Same as the PRN for students who joined before SRNs existed
program STUDENT_INFO.ProgramAbbreviation B.Tech. Was the full name (Bachelor of Technology); expansion is left to clients
branch STUDENT_INFO.Branch Computer Science and Engineering
semester STUDENT_INFO.ClassName Sem-8
section STUDENT_INFO.SectionName Section B
email STUDENT_INFO.Email john.doe@example.com
mobile STUDENT_INFO.Mobile 9876543210 Was phone
campusCode derived from campus 1 PES University (Ring Road) → 1, PES University (Electronic City) → 2; any other name → null, counted under unknown_campus_code
campus STUDENT_PHOTO.instituteName PES University (Ring Road) Was RR/EC
firstName new STUDENT_INFO.FirstName JOHN
middleName new STUDENT_INFO.MiddleName null
lastName new STUDENT_INFO.LastName DOE
branchShortCode new STUDENT_INFO.BranchAbbreviation CSE
gender new STUDENT_PHOTO.gender Male
dateOfBirth new derived from STUDENT_INFO.DateOfBirth 2002-01-31 PESU sends epoch milliseconds of midnight IST; converted in IST, since in UTC it would land on the previous day

The login response is used only to confirm the login and to get the token for the profile call; none of its copies of the profile's details are used.

  • No fallbacks: a value its one source lacks is null; no other block or response stands in. null and blank strings (what PESU's mobile API sends for a missing value) are read as no value; any other text, "NA" included, is returned as sent. A response whose STUDENT_INFO is missing or holds no usable value is a 422.
  • Requested fields are always present: any field can be null: when PESU has no value for it, doesn't send it, or sends it in an unexpected shape. A requested field is never left out; only fields not asked for in fields are absent, and fields come back in the table's order whatever order they're asked for in. (4.x instead returned the string "NA" for empty values and dropped fields it had no value for.)
  • Every field is treated the same: if PESU sends any field in an unexpected shape, or stops sending it, only that field is null and the rest of the profile still comes back. Only a change to the response's structure is a 422. Both cases are counted and logged by field name (see Metrics), so a change on PESU's side shows up on the dashboard. A field sent with no value isn't counted: that's a student without one.
  • Verified:
    • On accounts whose PRN and SRN differ, LoginId holds the PRN and SRN the SRN. On the test account, the login's loginId was the PRN whether it logged in by email, PRN, SRN or phone.
    • Semester, section and campus on four accounts, three at Ring Road and one at Electronic City: ClassName is the semester alone (Sem-8), and every campus name mapped to its code.
    • Missing values: across six accounts' login and profile responses and three saved ones, the fields we read marked a missing value only with null or "", never "NA". "NA" appeared only in free-text fields we don't read, such as parents' details. So it's returned as text, not turned into null: a student surnamed Na is stored as "NA".
    • Sessions: PESU accepts more than one session per account. Two logins at the same moment both returned a full profile, and an older login's token still fetched the profile after a newer login, so a second login doesn't end the first.
    • A non-student account (a librarian's) can log in, but PESU refuses its profile request with {"status": 403, "message": "Access Denied"}, which is reported as a 502 with the usual message.

Errors

Situation Status
PESU rejects the credentials (HTTP 401) 401 AuthenticationError
A username or password that isn't valid text, or any other invalid request 400
The login can't reach PESU, gets another status, an unexpected body, or a 200 that isn't SUCCESS 502 UpstreamError (new)
The profile call fails or PESU declines it, including PESU's error envelope sent with a 200 502 ProfileFetchError
The profile response can't be parsed (not JSON, or a block that isn't an object), or it reports success but its STUDENT_INFO is missing or holds no usable value 422 ProfileParseError

CSRFTokenError is removed. Every error answers with the fixed message the README and Swagger document. The specifics, such as the username and what PESU Academy answered, go only to the log, through the exception's new log-only detail.

Metrics

  • Removed: csrf_cache_total, csrf_refreshes_total and prefetch_tasks_total, with their JSON keys.
  • Upstream calls: labelled login and profile_fetch, so existing panels on those keep working.
  • profile_parse_errors_total reasons: response_structure (unparseable response, a 422), unknown_campus_code, and, counted once per field, missing_field (a field PESU didn't send) and unexpected_value (a field sent in an unexpected shape). For the last two, the caller gets null and a warning names the fields as PESU does, such as STUDENT_INFO.SRN, never their values.

Also in this PR

  • Dependencies: selectolax and the unused mdformat dev dependencies are removed, as is the CSRF token-expiry benchmark.
  • Coverage gate: now 100% of lines and branches over every Python file outside tests/ (app/, scripts/, .github/scripts/), configured once in [tool.coverage]. It was app/ only, by lines, at 95%.
  • Version check: the PR's version must now be exactly one step above dev's: the next patch, minor or major (from 4.18.0: 4.18.1, 4.19.0 or 5.0.0). It used to accept any higher version, so versions could be skipped. CONTRIBUTING has a new "Bump the Version" step.
  • Benchmark scripts: each script's body is moved into a main() so it can be tested, which also fixed a divide-by-zero when every parallel request failed.
  • Models: the upstream response models are in app/models/upstream.py, and ProfileField is next to ProfileModel in app/models/profile.py.
  • Docs: README, Swagger, CONTRIBUTING, SECURITY.md, the PR and issue templates, and .env.example.

🧱 Type of Change

  • 🐛 Bug fix – Non-breaking fix for a functional/logic error
  • ✨ New feature – Adds functionality without breaking existing APIs
  • ⚠️ Breaking change – Introduces backward-incompatible changes (API, schema, etc.)
  • 📝 Documentation update – README, docstrings, OpenAPI tags, etc.
  • 🧪 Test suite change – Adds/updates unit, functional, or integration tests
  • ⚙️ CI/CD pipeline update – Modifies GitHub Actions, pre-commit, or Docker build
  • 🧹 Code quality / Refactor – Improves structure, readability, or style (no functional changes)
  • 🐢 Performance improvement – Speeds up auth, profile retrieval, or reduces I/O
  • 🕵️ Debug/logging enhancement – Adds or improves logging/debug support
  • 🔧 Developer tooling – Scripts, benchmarks, local testing improvements
  • 🔒 Security fix – Addresses auth/session/data validation vulnerabilities
  • 🧰 Dependency update – Updates libraries in pyproject.toml, uv.lock

Breaking changes (major version, 5.0.0):

  • A profile field with no value is now null instead of being left out.
  • semester and section are null instead of "NA" for students who have graduated.
  • program is PESU's abbreviation (B.Tech.) instead of the full name (Bachelor of Technology).
  • phone is renamed mobile, PESU's own name for it.
  • campus is the campus's institute name (PES University (Ring Road)) instead of RR/EC; campusCode keeps its values but is mapped from that name rather than read from the PRN/SRN.
  • /metrics loses csrfCache, csrfRefreshes and prefetchTasks, the matching Prometheus families, and the csrf_fetch upstream operation.

The six new profile fields and the wider set of accepted fields values are additive.

🧪 How Has This Been Tested?

  • Unit Tests (tests/unit/)
  • Functional Tests (tests/functional/)
  • Integration Tests (tests/integration/)
  • Manual Testing

⚙️ Test Configuration:

  • OS: Linux
  • Python: 3.14 via uv
  • Docker build tested
  • Full run (scripts/run_tests.py with TEST_* credentials): 547 tests pass, 100% of 1066 statements and 162 branches, including the 13 live tests. Without credentials (what CI runs on PRs), coverage is also 100%.
  • Live tests, exact values: every profile field has a TEST_* variable and is compared exactly with the test account's real value, in both the all-fields and the specific-fields tests. A variable left empty means "expect null"; only the sign-in variables can't be empty. A mismatch names the fields without printing values.
  • Offline end to end: a wire fixture serves PESU's responses at the transport layer, so tests check the exact requests sent and drive /authenticate through the whole app for every upstream outcome.
  • Unit tests: every error path, each field's one source, that no other block or response stands in for a missing value, that unread blocks can't change a profile, that a field of an unexpected shape or one PESU doesn't send is null without failing the profile, and is counted, while one sent with no value isn't, client closing on every path including cancellation, concurrent logins staying isolated, and no password, token or private data in logs.
  • Docs tests: every Swagger example validates against its model, the documented errors match the real ones, the profile example lists every field, and the README's ProfileObject table lists every field in order with the model's type.
  • Mutation testing: dozens of deliberate bugs across the migration, the profile fields and the review fixes (following redirects, reading the date of birth in UTC, logging the raw response, not closing the client, dropping nulls, skipping the coverage gate and more) were each caught by at least one test.
  • Docker: the image builds and serves; benchmarked below. It's 216 MB, down from 232 MB on dev.

✅ Checklist

  • My code follows the CONTRIBUTING.md guidelines
  • I've performed a self-review of my changes
  • I've added/updated necessary comments and docstrings
  • I've updated relevant docs (README or endpoint docs)
  • No new warnings introduced
  • I've added tests to cover my changes
  • All tests pass locally (scripts/run_tests.py)
  • I've run linting and formatting (pre-commit run --all-files)
  • Docker image builds and runs correctly
  • Changes are backwards compatible (if applicable)
  • Feature flags or .env vars updated (if applicable)
  • I've tested across multiple environments (if applicable)
  • Benchmarks still meet expected performance (scripts/benchmark/benchmark_requests.py)

🛠️ Affected API Behaviour

  • app/app.py – Modified /authenticate route logic
  • app/pesu.py – Updated PESU Academy login or profile handling

🧩 Models

  • app/models/request.py – Input validation or request schema changes
  • app/models/response.py – Authentication response formatting
  • app/models/profile.py – Profile response schema

🐳 DevOps & Config

  • Dockerfile – Changes to base image or build process
  • .github/workflows/*.yml – CI/CD pipeline or deployment updates
  • pyproject.toml / uv.lock – Dependency version changes
  • .pre-commit-config.yaml – Linting or formatting hook changes

ci_checks.yml only gains the new TEST_* secrets for the test job, and passes TEST_MOBILE in place of TEST_PHONE.

📊 Benchmarks & Analysis

  • scripts/benchmark/benchmark_requests.py – Performance or latency measurement changes
  • scripts/benchmark/analyze_benchmark.py – Benchmark result analysis changes
  • scripts/run_tests.py – Custom test runner logic or behavior updates

📸 Screenshots / API Demos (if applicable)

A full profile response (example values):

{
  "status": true,
  "message": "Login successful.",
  "timestamp": "2024-07-28T22:30:10.103368+05:30",
  "profile": {
    "name": "John Doe", "prn": "PES1202000001", "srn": "PES1UG20CS001",
    "program": "B.Tech.", "branch": "Computer Science and Engineering",
    "semester": "Sem-2", "section": "Section C", "email": "johndoe@gmail.com", "mobile": "1234567890",
    "campusCode": 1, "campus": "PES University (Ring Road)", "firstName": "John", "middleName": null,
    "lastName": "Doe", "branchShortCode": "CSE", "gender": "Male", "dateOfBirth": "2002-01-31"
  }
}

Benchmark: dev vs this PR

scripts/benchmark/benchmark_requests.py --num-requests 100 (sequential, profile data on every request) against two Docker containers on the same machine, one after the other with the same test account. This PR was measured at an earlier commit, before the profile's fields were reworked; that work changes no upstream call, and both versions make the same two.

  • dev: pesu-auth:dev, built from upstream/dev at 485e9bd, serving 4.18.0.
  • This PR: pesu-auth:mobile-api, built from this branch, serving 5.0.0.
dev (web flow) This PR (mobile API) Change
Successful requests 100 / 100 100 / 100
Mean 3.978 s 3.041 s −23.6%
Median 3.930 s 3.023 s −23.1%
p90 4.143 s 3.240 s −21.8%
p95 4.201 s 3.411 s −18.8%
p99 4.709 s 4.045 s −14.1%
Min / max 3.842 / 5.084 s 2.709 / 4.876 s
Throughput 0.25 req/s 0.33 req/s +31%

Where the time goes, from each container's own /metrics (average seconds per call to PESU Academy):

Upstream call dev This PR
CSRF token prefetch 1.112 s (101 calls, in the background) none
Login 1.694 s 2.238 s
Profile 2.272 s 0.797 s
Calls to PESU per request 3 2

The mobile login is about 0.5 s slower than the web login, but the profile call is about 1.5 s faster, which more than makes up for it. dev also makes a third call per request, the CSRF prefetch, which runs in the background, so it costs PESU traffic rather than response time. Both containers opened and closed one HTTP client per request, with no errors.

Distribution of response times

Response time over requests

Benchmark: parallel requests, dev vs this PR

The same experiment against each branch's Docker image, one container at a time on the same machine, with the same test account:

  • The run: benchmark_requests.py --parallel --max-workers N, profile data on every request, a 30 s client timeout.
  • The images: dev is 485e9bd (4.18.0, the web flow); this PR is 5.0.0 (the mobile API).
  • Throughput is measured from wall-clock time. These runs used an earlier version of the script, whose "total time" added up the requests' latencies; in a parallel run those overlap, so it understated throughput. Since 9ddd6a3 the script records each request's start time and reports elapsed time and throughput itself, and analyze_benchmark.py computes throughput the same way.
Workers Requests dev: successful dev: throughput This PR: successful This PR: mean latency This PR: throughput
1 10 10 0.23 req/s 10 3.56 s 0.28 req/s
5 20 1 n/a 20 2.98 s 1.62 req/s
10 30 not run 30 2.98 s 3.11 req/s
20 40 not run 40 3.19 s 5.70 req/s
40 80 not run 80 3.46 s 10.28 req/s
80 160 not run 160 3.79 s (median 3.45 s, p95 5.27 s) 17.86 req/s
  • dev can't serve parallel logins for one account. At 5 workers, 1 of 20 requests succeeded. Its /metrics showed 12 401s, where valid credentials were rejected, and 7 422s (ProfileParseError, reason page_structure). The web flow allows one active session per account, so concurrent sign-ins break each other. The dev ramp stopped there rather than send more failing logins to PESU Academy.
  • This PR has no such limit. PESU Academy's mobile API accepts more than one session per account. Every request at every level succeeded, and throughput rises almost linearly with workers while latency barely moves, so one container is far from its limit at 80. The time is spent waiting on PESU: about 2.5 s for the login and 0.9 s for the profile call on average.
  • Its service stayed clean throughout. After all its runs, this PR's /metrics showed every response a 200, no errors, every HTTP client closed (485 created, 485 closed), and no profile parse errors.
  • Not pushed past 80 workers, to avoid loading PESU Academy with more logins on one account. The wall-clock times include a second or two of script start-up, so the lower rows slightly understate throughput.

🧠 Additional Notes (if applicable)

Before merging

  • Add the repository secrets the live tests now need. ci_checks.yml already passes them, but CI on dev will fail until they exist:

    • TEST_FIRST_NAME, TEST_MIDDLE_NAME, TEST_LAST_NAME, TEST_GENDER, TEST_DATE_OF_BIRTH;
    • TEST_BRANCH_SHORT_CODE, which is now required;
    • TEST_MOBILE, with TEST_PHONE's value, since every TEST_* variable is now named after its profile field; then delete TEST_PHONE;
    • and change TEST_PROGRAM to the abbreviation PESU sends (e.g. B.Tech.), and TEST_CAMPUS to the campus name (e.g. PES University (Ring Road)).
    • For a field the test account has no value for (e.g. TEST_MIDDLE_NAME with no middle name, TEST_SEMESTER and TEST_SECTION once graduated), don't create the secret, and delete it if it holds NA: a missing secret reads as empty, which means "expect null", while NA would now be expected as text.

    Pull-request CI doesn't run the live tests, so it stays green until then.

  • Conflict: fix: Covered the last untested branch and enforced branch coverage #238 also enforces branch coverage, so whichever merges second will need a rebase.

Before production

  • Semester, section and campus, verified on four accounts (three Ring Road, one Electronic City): STUDENT_INFO.ClassName is the semester alone (Sem-8), SectionName the section (Section B), and STUDENT_PHOTO.instituteName names the campus. A current student's profile (Sem-3, Electronic City) has the same formats. semester is ClassName as sent, so no format is parsed.
  • To check on staging: compare staging (this branch) with production (still the web flow) for one account. Every field should match apart from the intended changes above.

Follow-ups outside this PR

  • Grafana: the panels using the removed csrf_* and prefetch_tasks_total metrics will go empty and should be removed.
  • pesu-dev/skills: the agent docs in .agents/ still describe the web flow and need a separate PR:
    • upstream.md, architecture.md, metrics.md and testing.md;
    • api-contract.md: the 17 fields, program as an abbreviation, mobile, campus as the institute name, null rather than absent, and the error mapping;
    • security.md and AGENTS.md rule 1: the profile, including gender and date of birth, is logged at INFO;
    • known-issues.md: entries that are now fixed;
    • the update-scraper and change-upstream-client skills, and the scraper-specialist role.
  • Plots: the benchmark plots are hosted on pr-241-benchmarks, an image-only branch on the author's fork that isn't part of this PR; the links pin a commit.

🤖 Generated with Claude Code

…e API

Logging in by driving the PESU Academy web portal meant prefetching a CSRF token (a cached client,
a 45-minute refresh loop and background prefetch tasks), posting the login form and scraping the
profile page's HTML. Any markup change broke profiles with a 422, the CSRF machinery was the source
of most of the client leaks and races this module has had, and a profile request took about 4s.
Issue pesu-dev#233 proposes the mobile app's JSON API instead.

Each request now opens one HTTP client, logs in with a multipart POST to
/MAcademy/mobile/mobilelogin/auth, and, only if a profile was asked for, calls
/MAcademy/mobile/dispatcher (action 27, mode 1, menu 11172) with the access token from the login
body. The client is closed on every path, including cancellation, through the existing shielded
close. Nothing is cached between requests, so the lifespan no longer prefetches or refreshes
anything. These values were confirmed against the live API with the test account; the variant in
PR pesu-dev#152 (userId/searchUserId) is rejected by the dispatcher.

Responses are parsed into private pydantic models that declare only the fields we return. The
dispatcher response also carries photos, dates of birth, addresses, marks and parents' contact
details; they are dropped at parse time so no log, repr or exception can carry them, parse
failures log field locations only and are raised without their chained ValidationError, and the
access token is excluded from the model repr.

The profile comes mostly from STUDENT_INFO (SRN, registered name, full branch name), falls back to
STUDENT_PHOTO and the login response, and prefers the login for the current class and section.
The PRN is only taken from a loginId shaped like one, since upstream also puts SRNs there. Program
abbreviations are expanded to the full names the API has always returned; an unknown one is
returned as sent and counted. The campus now comes from the SRN, falling back to the PRN. A value
upstream does not have, including "" and the old "NA" placeholder, is returned as null, and
/authenticate now dumps with exclude_unset so requested fields are present (null when empty)
while filtered-out fields and an unrequested profile stay absent.

Error mapping: PESU's 401 is a 401; an unreachable login, any other status, an unexpected body or
a missing success marker is the new UpstreamError (502); a failed profile call is
ProfileFetchError (502); an unparseable profile, including one with neither student block, is
ProfileParseError (422). CSRFTokenError is gone.

Metrics: csrf_cache_total, csrf_refreshes_total and prefetch_tasks_total are removed, with their
JSON keys; the upstream operations are login and profile_fetch, so existing panels on those keep
working; profile_parse_errors_total reasons are now response_structure, unknown_program and
unknown_campus_code. selectolax and the CSRF token expiry benchmark are removed.

BREAKING CHANGE: profile fields with no value are now null rather than absent, semester and
section are null rather than "NA" for students not in a class, the /metrics JSON loses csrfCache,
csrfRefreshes and prefetchTasks (and the Prometheus output the matching families), and the
csrf_fetch upstream operation no longer exists.

Tested: the PESUAcademy unit tests are rewritten against real httpx2 responses shaped like the
live API, covering every error path, each mapping fallback, client closing on every path and the
absence of secrets in logs. The live tests gain SRN logins, and "NA" in a TEST_* profile value now
means "expect null". 306 tests pass with 100% coverage in one full run with credentials, against
a graduated test account whose PRN equals its SRN; current-student formats are not yet verified
live.
The README and the .github docs still described the web flow in places, and some of them were
wrong independently of it.

README: adds a "How it works" section (the mobile API, what happens to credentials and the access
token, and that the upstream API is undocumented); documents what request validation rejects,
that fields only applies with profile, and that a profile costs a second upstream call; clarifies
the 401 and 502 rows and notes that an unknown path or method gets the framework's own 404/405
body; adds types and examples to the ProfileObject table and states that any field can be null;
adds a fields example with a null value. Both full /metrics examples are regenerated with the real
renderers from one consistent set of numbers, matching the Swagger examples in app/docs/metrics.py,
since the previous ones no longer satisfied the accounting rules the README states.

.github: CONTRIBUTING explains the NA convention for TEST_* values, corrects the live-test count
(thirteen), says what a live-test failure usually means, warns that the benchmark makes real
logins, lists the sync-agents hook and points to SECURITY.md. SECURITY.md describes how
credentials and the extra personal data in PESU's responses are handled. The PR template asks for
a Conventional Commit title, as merged PRs use, and the bug template starts the API correctly,
lists /metrics and drops the nonexistent app/util.py option. Labels and checkboxes no longer
mention scraping.
Major, because the migration to the mobile API changes the response contract: profile fields
with no value are null instead of absent, and the CSRF metrics are removed from /metrics.
@aditeyabaral
aditeyabaral requested a review from a team as a code owner October 4, 2026 19:32
Copilot AI balanced review requested due to automatic review settings October 4, 2026 19:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

mdformat and mdformat-gfm were added to the dev group in pesu-dev#148, but nothing runs them from it: the
pre-commit hook installs its own mdformat with mdformat-gfm and mdformat-frontmatter, and no script
or workflow calls them. Running the dev-group copy directly would also rewrite the frontmatter of
the generated .github/agents/ files, since it lacks the frontmatter plugin.

Removes six packages from uv.lock. uv lock --check passes, the mdformat hook and the unit tests
still pass, and the version check is unaffected.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Profile responses can accept missing student data, and dispatcher failures can be misclassified as parse errors instead of upstream errors.

Review effort: Lite
Findings: 1 Medium severity · 1 Low severity

Open (2)

Comment thread app/pesu.py Outdated
Comment thread README.md Outdated
benchmark_requests.py and analyze_benchmark.py kept all their logic under
`if __name__ == "__main__":`, so no test could exercise it and it never appeared in a coverage
report. The argument parsing, the sequential and parallel runners, writing the CSV and the analysis
entry point are now functions, each script's main() takes an optional argv, and the entry guard only
calls main(). Command line behaviour is unchanged.

Also fixes a crash: when every request failed in the parallel runner, no time was recorded and the
average divided by zero after the results had been written. The average is now skipped when there
is nothing to average.
Coverage was measured over app/ alone, by lines only, with a 95% gate. scripts/run_tests.py,
.github/scripts/check_version_bump.py and both benchmark scripts had no tests, one branch in
app/pesu.py was untested, nothing drove /authenticate through the whole app with PESU mocked,
and tests/unit/test_readme.py asserted on a mock it had configured itself.

Coverage is now configured once in [tool.coverage] in pyproject.toml: app/, scripts/ and
.github/scripts/, with branch coverage and a 100% gate. scripts/run_tests.py passes a bare --cov, so
the hook, CI and a plain `pytest --cov` enforce the same thing. Only `if __name__ == "__main__":` is
excluded.

New tests:
- a `wire` fixture serving PESU responses at the transport layer, used to check the exact requests
  sent (multipart fields, X-Client-Type, the bearer token only on the profile call, the password
  only in the login, one client per login with a 10s timeout, redirects never followed);
- test_authenticate_flow.py: /authenticate end to end for every upstream outcome, checking status,
  body, the IST timestamp, metrics, that validation failures never reach PESU, and that no secret or
  upstream error text reaches a log or a response;
- PESUAcademy: concurrent logins stay isolated, cancellation during the profile call and during the
  close still closes the client, PRN shapes, extra upstream fields ignored and retyped ones rejected;
- OpenAPI: the documented profile lists every field, the filtered example matches its request, and
  the documented 422 and 502 messages are the real exceptions';
- the test runner, the version check and the benchmark scripts, all without network access;
- /readme against the real route.

404 tests pass in one full run with credentials, at 100% of 974 statements and 150 branches; the
suite without live tests, as CI runs it on pull requests, also reaches 100%. 21 deliberate bugs
(following redirects, logging the raw response, not closing the client, dropping nulls, skipping
the coverage gate and others) were each caught by at least one test.
The profile response carries more of the student's profile than the API returned. Ten fields are
added to ProfileObject, and to the defaults, so a request without `fields` gets them too:

- firstName, middleName, lastName: STUDENT_INFO, with STUDENT_PHOTO and the login's first name as
  fallbacks for the first name
- programShortCode: the abbreviation PESU sends ("B.Tech."), alongside the expanded program
- branchShortCode: BranchAbbreviation, falling back to the login's "Branch:CSE" without its prefix
- institute: STUDENT_PHOTO.instituteName
- rollNumber: from the latest entry of STUDENT_SEMESTERS by batchClassOrder, since roll numbers
  change between semesters and the list is not ordered
- gender, dateOfBirth, bloodGroup

The date of birth arrives as the epoch milliseconds of midnight IST, so it is converted in IST: in
UTC the same instant falls on the previous day. This was checked against the login's own
YYYY-MM-DD date for the test account. The login's string is the fallback, used only if it is
already in that format. STUDENT_SEMESTERS is optional like the student blocks, so a student without
semesters gets a null roll number rather than a 422.

Parents' details, addresses, marks and CGPA, admission data, the photo and internal IDs are still
not returned.

Tested: unit tests cover each field, every fallback, the IST conversion (including dates before
1970 and out-of-range timestamps), the latest-semester rule, and the Swagger examples matching the
model. The live tests check the original fields against TEST_* as before and the new ones for
consistency and format, so no new secrets are needed; TEST_BRANCH_SHORT_CODE is used when set. 427
tests pass in one full run with credentials, at 100% of lines and branches.
… 502

From the Copilot review of this PR.

Every field of STUDENT_INFO and STUDENT_PHOTO is optional and unknown keys are ignored, so `{}` --
which PESU does send for an empty block, as PLACEMENT_DETAILS shows -- parsed into an all-empty block
that passed the presence check. A profile response with no student data then returned a 200 with a
mostly-null profile instead of the documented 422. A block now counts only if it holds a value.

When the dispatcher rejects a request it answers HTTP 200 with its own envelope, such as
{"status": 400, "message": ...}. That was reported as a 422, which means PESU's API has changed,
when it is PESU refusing to serve the profile. It is now a ProfileFetchError (502), as the login
path already treats the equivalent case, with the status in the message and PESU's text not
forwarded. Bodies that are not such an envelope, including one with a status of 200, remain 422s.

Also corrects the README's fields example, whose SRN had the shape of a PRN.

Tested: 422 cases for empty blocks, blocks with only unknown or empty values and a status-200
envelope; a 502 for the error envelope that records no parse error; a profile built from one
non-empty block. Each fix was mutation-checked. 432 tests pass with credentials, at 100% of lines
and branches.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved critical import failure plus profile validation, sensitive logging, error mapping, and live-test credential-gating issues remain.

Review effort: Lite
Findings: 1 High severity · 3 Medium severity

Open (4)
Resolved since last review (2)

Comment thread app/pesu.py
Comment thread app/pesu.py Outdated
Comment thread tests/integration/test_app_integration.py
Comment thread tests/unit/test_run_tests.py Outdated
…on TEST_SRN

From the Copilot review of this PR.

A login answered with HTTP 200 and a status other than SUCCESS (for example "FAILURE") was reported
as a 401. Rejected credentials have only ever been seen as an HTTP 401, which is still mapped to
AuthenticationError; any other unsuccessful 200 is a response nobody has seen, so it is now an
UpstreamError (502), as a missing status already was. Telling users their password is wrong would
hide an upstream change as client errors.

scripts/run_tests.py decided whether to run the live tests from TEST_EMAIL, TEST_PRN, TEST_PHONE
and TEST_PASSWORD, but the SRN login tests added in this PR also need TEST_SRN. Without it they ran
and sent None as the username. TEST_SRN is now part of the check. CI already passes TEST_SRN to
the test job, so this does not switch the live tests off there.

Tested: the non-success statuses (FAILURE, a lower-case success, NA, null and a missing status) all
raise UpstreamError without a profile call, and the runner deselects the live tests when any one of
the five credentials is missing. 436 tests pass with credentials, at 100% of lines and branches.
PESU puts either ID under "loginId" in each block of its responses. The formats are:

- PRN: PES, the campus digit, the year of joining and a 5-digit number (PES1201800001)
- SRN: PES, the campus digit, the program (UG, PG, ...), the last two digits of the year of
  joining, the branch and a 3-digit number (PES2UG25CS001)

Students who joined before SRNs were introduced have an SRN that is their PRN. So the shapes never
collide: an all-digit ID is always the student's PRN, and one with letters is always their SRN.

The profile used to merge STUDENT_INFO's and STUDENT_PHOTO's loginId into one value before looking
for the PRN, keeping STUDENT_INFO's. If that was the SRN and STUDENT_PHOTO held the PRN, the PRN was
lost and `prn` came back null. The IDs are now kept apart and chosen by shape:

- prn: the first PRN-shaped value among the login's loginId, STUDENT_INFO's LoginId and SRN, and
  STUDENT_PHOTO's loginId
- srn: STUDENT_INFO.SRN as sent, as the one field labelled as the SRN; then any new-style SRN in
  the other ID fields; then STUDENT_PHOTO's loginId if it is a PRN (an older student's SRN).
  Something that is not an ID at all is no longer taken as the SRN.

The README, ProfileModel, .env.example and the code comments now describe both formats and the
older-student case. A comment that said older students "may only have a PRN" is corrected. The
Swagger example named basic_srn_auth now uses an SRN, and the live profile check asserts the IDs'
shapes and that they share a campus digit.

Tested: the SRN shape (including a three-letter branch, a four-digit year and lower case); the PRN
found in STUDENT_PHOTO; an older student's SRN doubling as the PRN; the SRN taken from the login; a
new-style SRN preferred over a PRN-shaped photo loginId; a non-ID ignored; the labelled SRN trusted
as sent. Each rule was mutation-checked. 451 tests pass with credentials, at 100% of lines and
branches.
…layout

The PRN/SRN layout has now been verified against accounts whose PRN and SRN differ. PESU sends the
PRN under the login's loginId, STUDENT_INFO's LoginId and USER_ROLE's LoginId, and the SRN under
STUDENT_INFO's SRN and STUDENT_PHOTO's loginId. The by-shape selection already picked both correctly
for that layout.

USER_ROLE's LoginId is now read too, as one more place to find the PRN, so a profile still gets its
PRN if the login and STUDENT_INFO stop carrying it. An absent, empty or unusual USER_ROLE is harmless.

The code comments and the test fixture now describe the verified layout rather than the notes in
issue pesu-dev#233 and PR pesu-dev#152, and the fixture carries a USER_ROLE block like the real response.

Tested: the PRN found in USER_ROLE alone, and an absent, empty, placeholder or numeric USER_ROLE;
the source was mutation-checked. 456 tests pass with credentials, at 100% of lines and branches.
…t, and stop reading blood group

From a review of this PR.

The fields added for the larger profile were strictly typed, so one of them in an unexpected shape
failed the whole profile with a 422: STUDENT_SEMESTERS as {} (how PESU already sends empty blocks),
USER_ROLE as a list, DateOfBirth as a string, a roll number with a letter. These secondary fields --
name parts, short codes, institute, roll number, gender, date of birth, USER_ROLE and the semesters,
each semester on its own -- now become null instead, and a warning names the field (never its value).
The core fields stay strict, so a change to them is still a 422.

When the latest semester has no usable roll number, rollNumber is now null rather than an earlier
semester's, which would be wrong rather than missing.

A username or password containing an unpaired surrogate decodes from JSON but cannot be encoded, and
crashed while the login request was built: a 500 logged at ERROR. RequestModel now rejects it as a
400, without the value in the message.

Blood group is no longer read or returned: it is not related to authentication and nothing uses it.
It is now among the response data that is discarded, like parents' details, and requesting
`bloodGroup` in `fields` is a 400. The README, ProfileModel and Swagger example are updated.

Tested: each secondary field of an unexpected shape costs only itself, in both responses; core fields
are still a 422; malformed semesters; the stricter roll-number rule; the 400 for undecodable text in
either field, with no request to PESU and nothing logged at ERROR; blood group neither returned,
requestable nor logged. Each fix was mutation-checked. 478 tests pass with credentials, at 100% of
lines and branches.
…s without sys.path

From a review of this PR.

scripts/run_tests.py had its docstring above the `#!` line, which made the shebang inert. It now
comes first, as in .github/scripts/check_version_bump.py.

tests/unit/test_benchmark.py put scripts/benchmark on sys.path for the whole test session and
imported the helper as a bare `util`, which a future module of that name would collide with. The
scripts are now loaded by path under names of their own; `util` is registered only while they load
and the previous entry is restored. A test checks that nothing is left behind.
…ive tests

The live tests compared the original eleven fields with TEST_* values but checked the newer ones
(name parts, short codes, institute, roll number, gender, date of birth) only for consistency and
format. Every default field now has a TEST_* variable and is compared exactly, with NA meaning the
account has no value, so the API must return null. A mismatch names the fields, never their values.
The ID shape checks stay, since they do not depend on the variables.

New variables: TEST_FIRST_NAME, TEST_MIDDLE_NAME, TEST_LAST_NAME, TEST_PROGRAM_SHORT_CODE,
TEST_INSTITUTE, TEST_ROLL_NUMBER, TEST_GENDER, TEST_DATE_OF_BIRTH. TEST_BRANCH_SHORT_CODE, which was
optional, is now required. .env.example documents them, CONTRIBUTING explains the NA convention for
any field, and ci_checks.yml passes them to the test job. The matching repository secrets have to
exist before the live tests run in CI on dev.

Tested: 478 tests pass with credentials, at 100% of lines and branches; a deliberately wrong
TEST_ROLL_NUMBER fails the live check and names only rollNumber.
…fields

The tests that name particular fields still named only the eleven the web flow returned, so the new
fields were checked only through the full profile. They are now checked the same way as the
original fields everywhere:

- The live specific-fields tests (functional and integration) request every new field alongside
  their original ones, and compare each with its TEST_* value through a shared check that names
  mismatched fields without printing values. The account's empty middle name also covers a
  requested field coming back null against the real API.
- A unit test fails if a profile field is added without being added to that live list.
- Field filtering, empty requested fields, duplicate fields and fields without a profile include new
  fields, and filtering is checked to return them in the documented order.
- The concurrency test gives each student their own first name, roll number and date of birth, so
  a mix-up between simultaneous logins would show in the new fields too.
- The end-to-end, metrics and null-serialisation tests include new fields.
- Request validation accepts every profile field, and rejects the snake_case forms of the new
  fields (as campus_code is rejected) and fields the API does not return (bloodGroup, photo,
  profilePicture), in both unit and integration tests.

The Swagger, request-model and README field examples now include new fields too.

Tested: 500 tests pass with credentials, at 100% of lines and branches. A wrong TEST_GENDER fails
the live specific-fields test naming only gender, and dropping a field from the live list fails the
guard.
… the current API

An audit of all documentation against the current behaviour.

Swagger (generated from the route docstring, the models and app/docs):
- /authenticate's description explains the credentials' handling, that `fields` only applies with
  `profile`, the field order, that an unknown field name is a 400, and when a field is null.
- ProfileModel's description, which Swagger shows for the profile, states the null rules and that
  the newer fields fail soft. The program, semester, section, campus, campusCode, branchShortCode and
  rollNumber descriptions give their format or when they are null.
- The request examples name the username form they use, and a PRN example joins the SRN, email and
  phone ones. The profile examples explain null fields and the field order, since Swagger cannot
  show a null value in an example.
- The 400, 401, 422 and 502 descriptions say what causes each; the request and response model
  descriptions for password, profile, fields, timestamp and profile are clearer.

README: the profile section explains the field order, which fields fail soft, when rollNumber is
null and that discarded data (now naming blood group) cannot be requested; the 422 row notes it
needs a profile request; the timestamp is described as IST; the password row says it is never
stored or logged.

.github: SECURITY.md names blood group among the discarded data; the feature-request template's
example, fetching the date of birth, is now implemented, so it suggests something that is not;
CONTRIBUTING explains what adding a profile field involves (TEST_* variable, the live tests' lists,
the repository secret, the README and ProfileModel).

No behaviour changes. The offline suite passes at 100% of lines and branches, including the
OpenAPI documentation tests.
…del as the error body too

From a check of every model and Swagger page against the code.

/metrics was summarised in Swagger as "Metrics Endpoint", from its function name, unlike Health,
Readme and Authenticate; it now has an explicit "Metrics" summary. ResponseModel is also the schema
of every documented error response, and its description now says so, along with what status,
message and profile hold in each case.

The rest checked out: every RequestModel and ProfileModel field has a description and an example
that validates; ProfileModel matches the field list and the default fields and every field is
optional; both /metrics examples, in Swagger and in the README, match the collector's HELP and TYPE
lines and validate against MetricsModel. The offline suite passes at 100% of lines and branches.
@aditeyabaral aditeyabaral changed the title feat!: authenticate and fetch profiles through the PESU Academy mobile API feat!: release 5.0.0 — authenticate and fetch profiles through the PESU Academy mobile API Oct 5, 2026
@aditeyabaral aditeyabaral changed the title feat!: release 5.0.0 — authenticate and fetch profiles through the PESU Academy mobile API feat!: release 5.0.0; authenticate and fetch profiles through the PESU Academy mobile API Oct 5, 2026
@aditeyabaral
aditeyabaral requested a lite review from Copilot October 5, 2026 01:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

This was referenced Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate authentication and profile retrieval to the PESU Academy mobile API

6 participants