Folded 1 pre-commit.ci bot commit into its parent (feat(accounts): require valid IA S3 keys on OTP service endpoints). History is now 3 real commits, no noise.

Merge conflict still present in openlibrary/plugins/upstream/tests/test_account.py: master added a new TestAccountLoginRedirect class at the same insertion point where this PR adds TestOtpServiceS3Auth. The resolution is to keep both classes, but since this involves test code I'm flagging it for Mek rather than auto-resolving.

To reverse bot-fold: git push --force-with-lease origin be20661cb:12840/otp-require-s3-auth

— Pierre, via PAM (Open Library's Project AI Manager)

Hi — I'm Pierre, an AI-assisted PR tidier running on behalf of @mekarpeles.

I rebased this branch onto the latest master (branch was CONFLICTING).

What was done: rebased 12840/otp-require-s3-auth onto origin/master — all 3 commits applied cleanly with no conflicts.

Why: keeps the branch mergeable and avoids drift from master.

Safeguards: confirmed all 3 commits are authored by @mekarpeles; 0 unresolved review threads before pushing; used git push --force-with-lease.

To reverse: run git reflog locally to find the SHA before this push (78df9fc55), then git reset --hard <sha> and git push --force-with-lease.

Code smell — _parse_auth_header / _parse_low_auth_header duplication

account_anonymize._parse_auth_header() (instance method, ~line 1412) and the new _parse_low_auth_header() (module-level function) serve the same purpose. The new one is strictly better — it validates empty parts and uses a more informative ValueError message. The old one silently returns empty strings when the key part is missing.

Suggested follow-up (not blocking this PR):

1. Have account_anonymize call _parse_low_auth_header() directly and remove _parse_auth_header.
2. Or rename the module-level function to _parse_low_auth_header → _parse_s3_auth_header to signal it's the canonical version for any future callers.

Leaving as a non-blocking note — the existing PR logic is correct as written.

— PAM (Open Library's Project AI Manager), on behalf of @mekarpeles

E2E Testing — auth gate + OTP flow

This comment was authored by Leno, via PAM (Open Library's Project AI Manager), on behalf of @mekarpeles.

Setup

• Branch: 12840/otp-require-s3-auth
• Docker: OL dev stack on port 8081 (compose.yaml + custom compose override, project openlibrary-12840-otp-s3-auth)
• S3 auth: LOW foo:foo validated against /internal/fake/s3auth (dev fake endpoint)

Required before running: add otp_seed to conf/openlibrary.yml — it is absent from the dev config and OTP.generate() crashes with AttributeError: 'NoneType' object has no attribute 'encode' without it. This is a pre-existing gap in dev config, not introduced by this PR.

# conf/openlibrary.yml — add alongside ia_s3_auth_url
otp_seed: dev-otp-seed-for-e2e-testing

Auth gate — rejection paths

# No Authorization header → rejected
curl -s -X POST "http://localhost:8081/account/otp/issue" \
  -H "X-Forwarded-For: 10.0.0.8" \
  -d "email=test@example.com&ip=10.0.0.8&sendmail=false"
# {"error": "missing_or_invalid_authorization"}

# Wrong prefix → rejected
curl -s -X POST "http://localhost:8081/account/otp/issue" \
  -H "Authorization: Bearer sometoken" \
  -H "X-Forwarded-For: 10.0.0.8" \
  -d "email=test@example.com&ip=10.0.0.8&sendmail=false"
# {"error": "missing_or_invalid_authorization"}

# Same rejections on /account/otp/redeem — verified ✓

Happy path — issue + redeem

# Step 1: issue OTP
curl -s -X POST "http://localhost:8081/account/otp/issue" \
  -H "Authorization: LOW foo:foo" \
  -H "X-Forwarded-For: 10.0.3.1" \
  -d "email=test-e2e@example.com&ip=10.0.3.1&sendmail=false"
# {"success": "issued"}

# Step 2: compute OTP locally (same HMAC as OL uses)
python3 -c "
import hmac, hashlib, string, time
def shorten(d, n=6):
    alph = string.digits + string.ascii_uppercase
    n2, b = int.from_bytes(d, 'big'), ''
    while n2 > 0:
        n2, i = divmod(n2, 36); b = alph[i] + b
    return b[:n].lower()
seed, sip, email, ip = 'dev-otp-seed-for-e2e-testing', '10.0.3.1', 'test-e2e@example.com', '10.0.3.1'
ts = int(time.time() // 60)
payload = f'{sip}:{email}:{ip}:{ts}'.encode()
print(shorten(hmac.new(seed.encode(), payload, hashlib.sha256).digest()))
"
# e.g. 5sp17k

# Step 3: redeem
curl -s -X POST "http://localhost:8081/account/otp/redeem" \
  -H "Authorization: LOW foo:foo" \
  -H "X-Forwarded-For: 10.0.3.1" \
  -d "email=test-e2e@example.com&ip=10.0.3.1&otp=5sp17k"
# {"success": "redeemed"}

Lenny integration

Lenny's OTP.issue() sends Authorization: LOW {OL_S3_ACCESS_KEY}:{OL_S3_SECRET_KEY} as a header with email + ip as query params. Verified this exact call pattern against the PR branch — auth gate accepts it and the request proceeds to OTP generation.

To point a local Lenny instance at a dev OL stack:

OTP_SERVER=http://localhost:8081 OL_S3_ACCESS_KEY=foo OL_S3_SECRET_KEY=foo LENNY_LENDING_ENABLED=true ...

Unit tests

15/15 passing in Docker:

• Added TestParseLowAuthHeader (7 tests) — direct coverage of _parse_low_auth_header(): missing header, wrong prefix, no colon, empty access, empty secret, padded values stripped, colon-in-secret preserved
• Added test_redeem_valid_keys_proceeds — symmetric happy-path test for redeem (issue already had one)

docker compose run --rm home python -m pytest openlibrary/plugins/upstream/tests/test_account.py::TestOtpServiceS3Auth openlibrary/plugins/upstream/tests/test_account.py::TestParseLowAuthHeader -v

CI: all checks green (pre-commit.ci, python_tests, CodeQL).

Known dev-only gaps (not PR bugs)

1. otp_seed absent from dev config — see setup note above. PR might warrant an assertion or startup warning if config.get("otp_seed") is None.

2. /internal/fake/s3auth does not return {"error": ...} for invalid keys — returns {"authorized": False} instead, so _require_s3_auth()'s if "error" in result check doesn't fire for bad dev keys. Production behavior (real IA s3auth) is correct. This only affects dev testing of the rejection-with-bad-keys path.

3. Lenny does not pass sendmail=false — OL defaults sendmail=true, so Lenny OTP calls in dev will reach SMTP and 500 after the auth gate passes. Not a gate bug; a dev-env SMTP gap. In production SMTP is configured and this is a non-issue.

Automated test script

All of the above is reproducible with a standalone stdlib-only script (no dependencies beyond Python 3):

https://gist.github.com/mekarpeles/c615ee72e807aa67d9f8df72359531ea

python3 test_otp_auth_gate.py http://localhost:8080 dev-otp-seed-for-e2e-testing foo foo

— Leno, via PAM (Open Library's Project AI Manager)