Skip to content

feat(portal): add EdFinancial servicer support (account number + DOB device verification, parser)#16

Open
shmuelsash wants to merge 4 commits into
mattebad:mainfrom
shmuelsash:feat/edfinancial-provider-support
Open

feat(portal): add EdFinancial servicer support (account number + DOB device verification, parser)#16
shmuelsash wants to merge 4 commits into
mattebad:mainfrom
shmuelsash:feat/edfinancial-provider-support

Conversation

@shmuelsash

@shmuelsash shmuelsash commented Jun 8, 2026

Copy link
Copy Markdown

Closes #8

What

Adds support for EdFinancial as a servicer provider. EdFinancial's portal differs from the existing Nelnet-style flow in two ways, and this PR handles both.
Every change is gated to provider == "edfinancial", so other servicers (Nelnet, Aidvantage, MOHELA, etc.) are completely unaffected.

Why (addresses #8)

As reported in #8, two things break the existing flow for EdFinancial:

  1. New-device identity challenge. On a cookie-less / unrecognized device, EdFinancial does not email an MFA code. Instead it shows a step-up page asking for (account number or SSN) and date of birth, so the login form's username field is never found and the email MFA poller waits for a code that never arrives.
  2. Different portal structure. After login the authenticated app is server-rendered at myaccount.edfinancial.studentaid.gov, not the Nelnet-style Group: layout, so the existing loan/payment parsing finds nothing.

Per the maintainer's note in #8 (no SSN-centric workflow), account number + DOB is the primary, documented path. SSN is supported only as an optional, repr-hidden fallback (it's never required and is not the default).

What changed

Device verification (client.py, selectors.py, config.py)

  • New _maybe_complete_device_verification fills and submits the challenge, then proceeds to the dashboard. It is a no-op for any non-edfinancial provider.
  • Selectors targeted (from the challenge page in Edfinancial - 2FA via. Account Number / DoB #8):
    • Account number: #account-number / input[name="taccountNumber"]
    • SSN (optional fallback): input[name="tSSN1"], tSSN2, tSSN3
    • Date of birth: #dob1/input[name="tmonth"], #dob2/input[name="tday"], input[name="tyear"]
    • Submit: #Submit
    • These fields are keystroke-masked, so the automation types character-by-character rather than using fill.
  • After the challenge, login can land directly on the dashboard, whose numeric inputs can look like a code field. Added an edfinancial-gated "already logged in" guard so a logged-in page is not misread as an MFA prompt. For other providers this guard is always false, so their MFA path is unchanged.
  • The portal's click-through agree page before the login prompt is handled by the existing post-login readiness wait.
  • New optional config: SERVICER_ACCOUNT_NUMBER, SERVICER_DOB, and (optional) SERVICER_SSN, all repr-hidden so they do not leak into logs.

EdFinancial parser (client.py)

  • discover_loan_groups, _extract_loans, and _extract_payment_allocations each branch to an _edf_* implementation only for edfinancial; the existing code path is untouched.
  • Balances, rates, and due dates are parsed from /Loans/AllLoanDetails.
  • Per-loan payment allocations are parsed from /AccountHistory/ViewHistory using the "By Loan" / "Life of Loan" view. That view prepends an expand/disclosure cell that shifts every column, so the row parser locates the real date cell and maps columns relative to it (this also skips detail/expansion rows, avoiding double-counting).

Docs/templates

  • env.example, portal.env.example, and README.md document the new variables and add a troubleshooting entry for the new-device challenge.

Safety / blast radius

  • All behavioral changes are behind a provider == "edfinancial" (or != "edfinancial") check derived from the portal host. No existing provider's behavior changes.
  • The new config fields default to empty and are optional. SSN is never required.

Testing

Validated read-only against a live EdFinancial account (no writes to Monarch):

  • Loan-group discovery returns the correct groups.
  • Loan balances, interest rates, and due dates match the portal.
  • Per-loan payment allocations parse correctly, and the per-date splits reconcile to the expected monthly autopay total.

Notes

  • Only read-only paths were exercised; the balance/transaction-writing sync was not run against a live Monarch account in this validation.
  • Happy to share sanitized selector/flow detail on Edfinancial - 2FA via. Account Number / DoB #8 if useful; raw portal HTML is omitted since it contains account PII.

shmuelsash and others added 2 commits June 8, 2026 15:37
…device verification, parser)

Closes mattebad#8

## What

Adds support for **EdFinancial** as a servicer provider. EdFinancial's portal
differs from the existing Nelnet-style flow in two ways, and this PR handles both.
Every change is gated to `provider == "edfinancial"`, so other servicers
(Nelnet, Aidvantage, MOHELA, etc.) are completely unaffected.

## Why (addresses mattebad#8)

As reported in mattebad#8, two things break the existing flow for EdFinancial:

1. **New-device identity challenge.** On a cookie-less / unrecognized device, EdFinancial
   does not email an MFA code. Instead it shows a step-up page asking for (account number
   **or** SSN) and date of birth, so the login form's username field is never found and the
   email MFA poller waits for a code that never arrives.
2. **Different portal structure.** After login the authenticated app is server-rendered at
   `myaccount.edfinancial.studentaid.gov`, not the Nelnet-style `Group:` layout, so the
   existing loan/payment parsing finds nothing.

Per the maintainer's note in mattebad#8 (no SSN-centric workflow), **account number + DOB is the
primary, documented path**. SSN is supported only as an optional, repr-hidden fallback — it
is never required and is not the default.

## What changed

**Device verification** (`client.py`, `selectors.py`, `config.py`)
- New `_maybe_complete_device_verification` fills and submits the challenge, then proceeds to
  the dashboard. It is a no-op for any non-edfinancial provider.
- Selectors targeted (from the challenge page in mattebad#8):
  - Account number: `#account-number` / `input[name="taccountNumber"]`
  - SSN (optional fallback): `input[name="tSSN1"]`, `tSSN2`, `tSSN3`
  - Date of birth: `#dob1`/`input[name="tmonth"]`, `#dob2`/`input[name="tday"]`, `input[name="tyear"]`
  - Submit: `#Submit`
  - These fields are keystroke-masked, so the automation types character-by-character rather
    than using `fill`.
- After the challenge, login can land directly on the dashboard, whose numeric inputs can look
  like a code field. Added an edfinancial-gated "already logged in" guard so a logged-in page
  is not misread as an MFA prompt. For other providers this guard is always false, so their MFA
  path is unchanged.
- The portal's click-through agree page before the login prompt is handled by the existing
  post-login readiness wait.
- New optional config: `SERVICER_ACCOUNT_NUMBER`, `SERVICER_DOB`, and (optional)
  `SERVICER_SSN`, all repr-hidden so they do not leak into logs. Passing the challenge once
  persists a trusted session and the portal generally stops challenging (~90 days).

**EdFinancial parser** (`client.py`)
- `discover_loan_groups`, `_extract_loans`, and `_extract_payment_allocations` each branch to
  an `_edf_*` implementation only for edfinancial; the existing code path is untouched.
- Balances, rates, and due dates are parsed from `/Loans/AllLoanDetails`.
- Per-loan payment allocations are parsed from `/AccountHistory/ViewHistory` using the
  "By Loan" / "Life of Loan" view. That view prepends an expand/disclosure cell that shifts
  every column, so the row parser locates the real date cell and maps columns relative to it
  (this also skips detail/expansion rows, avoiding double-counting).

**Docs/templates**
- `env.example`, `portal.env.example`, and `README.md` document the new variables and add a
  troubleshooting entry for the new-device challenge.

## Safety / blast radius

- All behavioral changes are behind a `provider == "edfinancial"` (or `!= "edfinancial"`)
  check derived from the portal host. No existing provider's behavior changes.
- The new config fields default to empty and are optional. SSN is never required.

## Testing

Validated read-only against a live EdFinancial account (no writes to Monarch):
- Loan-group discovery returns the correct groups.
- Loan balances, interest rates, and due dates match the portal.
- Per-loan payment allocations parse correctly, and the per-date splits reconcile to the
  expected monthly autopay total.

## Notes

- Only read-only paths were exercised; the balance/transaction-writing `sync` was not run
  against a live Monarch account in this validation.
- Happy to share sanitized selector/flow detail on mattebad#8 if useful; raw portal HTML is omitted
  since it contains account PII.
…chant

Manually-entered transactions may use a different merchant name than the
sync-created ones (e.g. "EdFinancial" vs "Student Loan Payment"), causing
the guard to miss existing transactions and create duplicates. Since the
guard already scopes by account_id, date+amount is specific enough to
identify a duplicate payment on a loan account.

Adds require_merchant_match=False default; callers can opt back in to
strict matching if needed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mattebad

Copy link
Copy Markdown
Owner

Hey thank you for the contribution. I should have some time this week to check this out and review

@mattebad

mattebad commented Jun 12, 2026

Copy link
Copy Markdown
Owner

Thanks for the EdFinancial work here — this is very close, and I’d like to get it over the line with a few targeted changes.

Requested changes before merge:

  1. Duplicate guard safety behavior
  • Please keep find_duplicate_transaction strict by default (match on date + amount + merchant).
  • I’m okay supporting loose matching, but only as explicit opt-in (via config/env), not default behavior.
  • In loose mode, please still try strict first, then fall back to date + amount only if strict misses.
  • Please log clearly when a loose-match fallback is used to skip create.
  1. EdFinancial SSN fallback behavior
  • Please implement a true fallback flow:
    • Try account number + DOB first (if account number configured).
    • If challenge remains / attempt fails, auto-retry with SSN + DOB when SSN is configured.
    • If SSN is not configured, fail fast with explicit actionable error.
  • Current logic appears to choose account number OR SSN up front, but does not retry with SSN if account number path fails.
  1. Add tests for new provider path and guardrail changes
  • Add targeted unit tests for EdFinancial parsing/device-verification helpers (_edf_*, DOB parsing, challenge handling decisions).
  • Add tests for duplicate-guard semantics to ensure strict merchant matching remains default-safe behavior, with optional loose-mode fallback behavior covered.
  • Optional but helpful: add an EdFinancial-gated portal smoke path similar to existing provider smoke tests.
  1. Minor code quality cleanup
  • Please fix the indentation/readability nit in the new PortalCredentials(...) blocks in cli.py.

I’m very supportive of adding EdFinancial, and the provider-gated approach is solid. Main things are preserving idempotency safety and making fallback behavior deterministic.

@mattebad

Copy link
Copy Markdown
Owner

Quick clarification on duplicate guard behavior preference:

  • Default behavior should remain strict (date + amount + merchant).
  • If we support loose matching, it should be explicit opt-in only (e.g. env/config flag).
  • In loose mode, please still try strict match first, then fall back to date + amount only if strict match misses.
  • Please add a clear log line when a loose-match fallback is used to skip create.

I’m good with opt-in loose mode for users who intentionally edit merchant names, but I don’t want loose matching as default behavior.

…SSN fallback, tests

Duplicate guard:
- Restore strict (date + amount + merchant) as the default behavior.
- Add opt-in MONARCH_DUPLICATE_GUARD_LOOSE_MATCH; loose mode tries strict
  first and falls back to date+amount only if strict misses, logging clearly
  when a loose match is used to skip a create.

EdFinancial device verification:
- Replace up-front account#/SSN pick with a true fallback: try account number
  + DOB first, auto-retry with SSN + DOB if the challenge does not clear, and
  fail fast with an actionable error when neither is configured/clears.

Tests + docs:
- Add unit tests for duplicate-guard semantics and EdFinancial device
  verification (_parse_dob + full fallback matrix); add EdFinancial-gated
  portal smoke case.
- Fix PortalCredentials indentation nit in cli.py.
- Document the new knobs in README, env.example, portal.env.example,
  config.example.yaml.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@shmuelsash

Copy link
Copy Markdown
Author

Thanks for the detailed review! I've pushed changes addressing all four points.

1. Duplicate guard — strict by default, loose as opt-in

Reworked find_duplicate_transaction so the default is strict again (date + amount + merchant). My earlier "relax" commit is now gated behind an explicit opt-in flag:

  • New MONARCH_DUPLICATE_GUARD_LOOSE_MATCH / monarch.duplicate_guard_loose_match (default false).
  • In loose mode it still tries a strict match first and only falls back to date + amount (ignoring merchant) if strict misses — single pass, so a strict hit on a later page still wins over an earlier loose candidate.
  • When a loose fallback is used to skip a create, it logs a clear line: Duplicate guard: no strict date+amount+merchant match; using LOOSE date+amount match txn id=… (merchant differs: got … want …).

The post-create recovery lookup stays strict (it's matching a txn we just created with the exact merchant).

2. EdFinancial — true account# → SSN fallback

Replaced the up-front "account# OR SSN" pick with a real fallback flow:

  • Try account number + DOB first (when configured).
  • If the challenge doesn't clear, automatically retry with SSN + DOB (when configured) — "didn't clear" is detected by the device-verification form still being present after submit.
  • If neither clears it (or only the account number is configured and it fails), fail fast with an actionable error naming what to set.

Factored the fill/submit into a _edf_submit_device_challenge(...) -> bool seam and the orchestration into _complete_device_challenge_with_fallback(...).

3. Tests

  • tests/test_duplicate_guard.py — strict match, strict-default merchant-mismatch (no dup), loose fallback, loose-still-prefers-strict, no-candidate, amount-mismatch.
  • tests/test_edf_device_verification.py_parse_dob formats/boundary/malformed, plus the full fallback matrix (account-success, account→SSN retry, account-fail-no-SSN fails fast, SSN-only fail, both-fail) and the cleared/not-cleared detection.
  • Added an EdFinancial-gated case to test_portal_smoke.py matching the existing provider smoke pattern.

Full suite: 69 passed, 3 skipped (the skips are the credential-gated portal smoke tests).

4. Code quality

Fixed the indentation in the PortalCredentials(...) block in cli.py.

Also documented the two new knobs (MONARCH_DUPLICATE_GUARD_LOOSE_MATCH and the SSN fallback) in the README, env.example, portal.env.example, and config.example.yaml. Let me know if you'd like any of these tweaked.

@mattebad

Copy link
Copy Markdown
Owner

Great, I think this looks merge ready at this point bar 1 nitpick on my end.

Can we just add a SSN length guard, so if a user accidentally misses a number we don't waste their time running the automation only to find out they fat fingered an extra number or missed one.

Address PR mattebad#16 review: validate that an optional SERVICER_SSN, if set,
is exactly 9 digits (ignoring dashes/spaces) at config load instead of
failing mid-run on the portal device-verification challenge. A fat-fingered
extra/missing digit now raises a clear error before any browser launches.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@shmuelsash

Copy link
Copy Markdown
Author

Added a length guard in the ServicerConfig model validator — if SERVICER_SSN is set, it strips separators (dashes/spaces) and rejects anything that isn't exactly 9 digits with a clear error at config load, before any browser spins up. Formats like 123-45-6789 still work fine. Covered by new tests in test_config.py.

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.

Edfinancial - 2FA via. Account Number / DoB

2 participants