Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a Time-based One Time Password (TOTP) authentication system for Lenny services, providing email-based OTP flow with built-in security safeguards and rate limiting.
- Adds new
/account/otp/issueand/account/otp/redeemendpoints for OTP generation and verification - Implements rate limiting and service verification to prevent abuse
- Introduces HMAC-based OTP generation with configurable seed and time-based validation
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| openlibrary/plugins/upstream/account.py | Adds two new endpoint classes for OTP issue and redeem functionality |
| openlibrary/core/lending.py | Adds OTP seed configuration setup |
| openlibrary/core/auth.py | Implements the core TimedOneTimePassword class with generation, validation, and rate limiting |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
The OTP challenge needs work: tested with Got |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 11 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| payload = f"{service_ip}:{client_email}:{client_ip}:{ts}".encode() | ||
| digest = hmac.new(seed.encode('utf-8'), payload, hashlib.sha256).digest() | ||
| return cls.shorten(digest) | ||
|
|
There was a problem hiding this comment.
The function can raise socket.gaierror if hostname resolution fails, and AttributeError if parsed.hostname is None. Add error handling to prevent crashes.
| if not parsed.hostname: | |
| return False | |
| try: | |
| resolved_ip = socket.gethostbyname(parsed.hostname) | |
| except socket.gaierror: | |
| return False |
| if i.challenge_url and not OTP.verify_service(i.service_ip, i.challenge_url): | ||
| return delegate.RawText(json.dumps({"error": "challenge_failed"})) |
There was a problem hiding this comment.
If the challenge mechanism doesn't work due to infrastructure limitations, consider removing this code or clearly documenting plans to fix it. Dead code paths reduce maintainability.
- Move OTP/TOTP docs into docs/ai/subsystems/otp_service.md - Remove network-dependent doctest from otp_service_issue.POST (test env blocks network requests; doctest had no assertions anyway)
Closes ArchiveLabs/lenny#95
Prototype to implement Time bound One Time Passwords for Lenny services
Introduces an email-based One Time Password (OTP) flow with built-in safeguards.
Endpoints
Issue endpoint (
/account/otp/issue)(service_ip, email, client_ip, timestamp)sendmail=trueflag to email the OTP directly to the userRedeem endpoint (
/account/otp/redeem)Security & Abuse Prevention
Rate limiting
Challenge check
challenge_urltest (verifying it is a running Lenny server with https that returns json)Testing
From a Lenny service (must be running https)
Screenshot
Stakeholders