OAuth 2.0 authorization server: authorization code + PKCE - #211
Draft
mekarpeles wants to merge 13 commits into
Draft
OAuth 2.0 authorization server: authorization code + PKCE#211mekarpeles wants to merge 13 commits into
mekarpeles wants to merge 13 commits into
Conversation
Member
Author
M1–M3 done, adversarial review round 1 applied392 passed on real Postgres, 390 on SQLite (concurrency tests skip there), plus 5 Playwright tests and the mock consumer green end to end. The review earned its keepAn adversarial pass over M1 found bugs that 74 passing tests did not. Two were invisible by construction: the suite ran on SQLite, where each connection gets its own in-memory database, so two "concurrent" callers never contend and a double-spend cannot be observed.
Bugs the other layers could not find
Two findings deliberately deferred
Both are worth doing before this leaves draft. Still to come (M4)
Reviewers: |
Member
Author
OL + Lenny verified end to end, against the live feedTwo stages, because Open Library requires Python 3.14 and Lenny runs 3.12 — they cannot share an interpreter, which is honest given they are two services. Uses Open Library's real harvester against the live lennyforlibraries.org feed, then Lenny's real OAuth 2.0 server: Step 8 is the thing Open Library could not do at all before — it had no way to answer "what does this patron have on loan?" without either holding a credential it should not, or a key that reads every patron. Step 1 depends on internetarchive/openlibrary#13561 (green, Note step 2: the OPDS auth document still advertises only Since the last update
Open questions for @mekarpeles
Adversarial review round 2 (endpoints) is running; I will apply what it finds. |
Member
Author
Adversarial review round 2 — found a lending-policy bypassRound 2 went after the endpoints, which round 1 had only glanced at. It found something worse than round 1 did. CRITICAL —
|
| Guarantee | Item.borrow |
what /oauth2/borrow did |
|---|---|---|
| open-access items are not lendable | ✅ | ❌ |
| per-patron concurrent loan limit | ✅ | ❌ |
| per-item copy count | ✅ | ❌ |
SELECT … FOR UPDATE on the Item row |
✅ | ❌ |
Verified before fixing: an open-access item lent (201), a limit of 2 producing 5 loans, and a single copy lent to two different patrons at once — which also drives available_copies negative, so the OPDS feed would have lied about availability too.
That is a controlled-digital-lending violation reachable by any client a patron grants borrow to.
It survived because no test covered a successful borrow at all — the only borrow tests used a nonexistent edition or stopped at a scope refusal. Item.borrow now takes hashed=True, mirroring Loan.exists/Loan.create, since the token holds only the hash.
HIGH — the consent handle was replayable, and "Not now" was not final
No id on the handle meant one consent click could mint an unbounded number of grants for ten minutes — and replaying the handle with decision=allow after a denial issued a code anyway. Handles now carry a random id, spent on use, whichever way the patron decided.
One honest caveat: the spent-id store is per-process, so with several uvicorn workers a replay could land elsewhere. The proper fix is a table. It is commented in the code rather than buried, and belongs on #209.
HIGH — credentialed CORS wildcard over the consent screen
allow_origin_regex=".*" + allow_credentials=True reflected any origin on a cookie-authenticated page rendering the handle: evil.example could read it and POST it back — an authorization code with no click. Only SameSite=Lax stood in the way, and the OAuth code does not own that cookie. The consent endpoint now refuses cross-origin reads, from a middleware layered outside CORSMiddleware so it actually wins.
Also fixed
X-Frame-Options/frame-ancestors on consent (RFC 6749 §10.13) · iss on the authorization response and in metadata (RFC 9207 mix-up defence — which matters because this assumes many nodes) · client_id_issued_at/client_secret_expires_at (RFC 7591 §3.2.1, required) · WWW-Authenticate on invalid_client (RFC 6749 §5.2) · redirect URIs with a fragment now refused (RFC 6749 §3.1.2 — the code would land after the # and never reach the client) or control characters.
Plus, from earlier today
OAuthClient.disable() and sweep_expired() — an operator had no way to stop an abusive client under open registration, and the tables grew forever.
Test hygiene worth calling out
The fixture now clears loans. Lending policy counts a patron's active loans, so one left behind changed what the next test was allowed to do — the borrow tests passed alone and failed in a group. Order-dependence is indistinguishable from a real defect at 2am.
The borrow-policy tests are Postgres-only: SQLite neither autoincrements the BigInteger keys nor implements SELECT FOR UPDATE, so "passing" there would mean nothing.
Status
416 passed on Postgres, 410 on SQLite, 5 Playwright, mock consumer green, OL↔Lenny arc green.
Still open from round 2, for @mekarpeles to weigh
- H5 — a TOCTOU window after the atomic claim. The winner's token is created after the claim commits, so a loser detecting reuse in that window revokes nothing. Realistic on the refresh path (no PKCE verifier needed). The fix is to claim-and-issue in one transaction; I did not want to restructure transaction ownership without your eyes on it.
- H3 — the metadata issuer follows the
Hostheader whenLENNY_PROXYis unset. RFC 8414 §2 requires anhttpsissuer and §3.3 tells consumers to reject a mismatch, so a strict consumer must reject any node that has not setLENNY_PROXY. Needs an operator-facing decision: require the setting, or add a host allowlist. - M4 —
/.well-known/oauth-authorization-serverreturns 404 in Docker. There is no nginxlocationfor it. The discovery story does not work in the shipped deployment; my tests drive the ASGI app directly and never caught it.
Lenny becomes an authorization server so a consumer can act on a patron's behalf without either side holding credentials it should not. Scaffolding only — tests come next and will lead the remaining work. Opening the PR early so the design can be argued before more is built on it. Adds: lenny/core/oauth2.py clients, authorization codes, access/refresh tokens lenny/routes/oauth2.py authorize, token, revoke, register, loans, borrow /.well-known/oauth-authorization-server RFC 8414 metadata alembic a7c4e91d2f80 three tables, secrets stored as SHA-256 digests oauth2_consent.html patron-facing consent screen scripts/mock_openlibrary.py a stand-in consumer driving the whole flow Design notes in docs/plans/oauth2-authorization-server.md. Existing /oauth/* OPDS routes are untouched; this is additive. Refs #209 Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
74 tests across the core primitives and the HTTP endpoints. Every test named `test_attack_*` asserts a specific attack fails, so a green-to-red there means a defence was removed rather than a refactor went wrong. Covered: PKCE (S256 only, `plain` refused), single-use codes, code binding to client/redirect/challenge, code reuse revoking the tokens it produced, refresh rotation, scope escalation, bearer scope enforcement, revocation, and the RFC 6749 §4.1.2.1 rule that an unvalidated redirect_uri must render an error rather than redirect to it — otherwise /authorize is an open redirector. Two bugs the tests found, both real: - BigInteger primary keys do not autoincrement on SQLite (only INTEGER PRIMARY KEY does), so every insert failed there while working on Postgres. Fixed with `.with_variant(Integer, "sqlite")` so one model serves both. - Expiry checks compared a stored timestamp against an aware `now()`. Postgres `timestamptz` round-trips as aware, SQLite drops the tzinfo, so the same code raised TypeError on one backend and not the other. Expiry now goes through `_as_utc()` instead of assuming a backend. Also fixes a latent issue in core/db.py: an in-memory SQLite database lives inside one connection, so the default pool hands each checkout a fresh empty database. The app's `db_session.remove()` teardown returns a connection after every request, which made the schema appear to vanish mid-test. StaticPool under TESTING only; Postgres is untouched. Full suite: 367 passed, 13 skipped. Refs #209 Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
The suite now runs against real Postgres as well as SQLite (374 passed on both), the alembic migration is verified on Postgres including a down/up round-trip, and five Playwright tests drive the consent screen in Chromium. Two real bugs, neither reachable from unit or integration tests: 1. Discovery advertised the wrong endpoints. The RFC 8414 document was built from configured HOST/PORT, which default to localhost:8080, so any node not bound there published endpoints that do not exist — a consumer following discovery went nowhere, silently. Now derived from the request, with an operator-configured LENNY_PROXY still winning since that is the deployment speaking and the only thing that survives a proxy rewriting Host. Found by running the mock consumer against a node on another port. 2. Registration refused every loopback redirect_uri, so nobody could develop a client against a local node. RFC 8252 §7.3 exempts loopback: the code never crosses a network. Accepts http on 127.0.0.1/::1/localhost, compared against the parsed hostname — `127.0.0.1.evil.com` and `http://127.0.0.1@evil.com/` are refused, which a substring check would not have caught. Playwright covers what TestClient cannot: that the consent screen names the client and describes scopes in words, that Allow round-trips `state`, that Deny hands out no code, that an anonymous patron is sent to log in first, and that an unregistered redirect_uri renders an error rather than sending the browser there. The e2e fixtures also check up front that the server shares pytest's LENNY_SEED — a mismatch rejects every minted cookie and surfaces as an unrelated-looking `lending_not_configured`, which cost time to diagnose. tests/e2e/README.md documents the whole setup. Refs #209 Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
An adversarial review of M1 found bugs that 74 passing tests did not. The two worst were invisible by construction: the suite runs on SQLite, where each connection gets its own in-memory database, so two "concurrent" callers never contend and a double-spend cannot be observed. CRITICAL — authorization codes and refresh tokens could each be spent twice. Both used check-then-write. Under READ COMMITTED two callers both read the row as unspent, both pass every check, and both write. An attacker racing the legitimate client got a live token pair, and reuse detection never fired because neither caller saw the row as spent. The app runs three uvicorn workers by default, so this was reachable in production. Both now claim atomically with UPDATE ... WHERE <unspent> and check the rowcount, letting the database decide. Covered by tests/test_oauth2_concurrency.py, which drives two real threads through the real code on real Postgres and fails on the old code. HIGH — detected refresh-token reuse revoked nothing. Rotation only punished whoever moved second: an attacker who refreshed *first* kept a working token for the full 90-day lifetime while the victim silently re-authorized. Reuse now revokes the whole family (RFC 9700 §4.14.2, OAuth 2.1 §6.1). MEDIUM-HIGH — any client could revoke a victim's tokens with a spent code. The reuse branch ran before the client/redirect/PKCE checks, so replaying a spent code — which is not secret; it sits in browser history, Referer headers and consumer access logs — as a throwaway registered client destroyed the real client's tokens. Ownership is now established before reuse is acted on. MEDIUM — verify_pkce raised UnicodeEncodeError on a non-ASCII verifier (a 500 instead of invalid_grant) and accepted a 4-character one. RFC 7636 §4.1 mandates 43-128 unreserved ASCII; the 43 floor *is* the entropy requirement that makes an intercepted code unusable. MEDIUM — revoking a refresh token left access tokens from the same grant alive (RFC 7009 §2.1), so a patron withdrawing access stayed connected if the consumer had rotated. Revocation now covers the grant. MEDIUM — /oauth2/revoke declared client credentials and ignored them, letting any caller revoke any token they came to hold. It now authenticates the client (§2.1) and revokes only that client's tokens (§5), still always returning 200 so it cannot be used as an existence oracle. MEDIUM — revoke_for_code(None) compiled to `IS NULL` and revoked every token with no grant recorded, across all patrons. Now refuses. Also bounds registration input: unauthenticated, and a value longer than its column turned a validation failure into a 500. Two review findings deliberately left for follow-up, noted on the PR: consent phishing via unverified client_name on open registration, and the absence of a CSRF token on the consent POST. 392 passed on Postgres, 390 on SQLite (concurrency tests skip), mock consumer green end to end. Refs #209 Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
Both were the review's remaining open items, and the CSRF fix turned out to
also be the simpler design.
The consent form now carries one opaque, signed handle instead of the request's
parameters. Everything the POST acts on comes from that handle, so:
- it cannot be fed a different client_id, scope or code_challenge than the
patron was actually shown — previously all six were re-submitted as form
fields, i.e. attacker-shapeable;
- the handle is bound to the patron it was rendered for, so an attacker who
mints one by starting their own authorization cannot get a victim's browser
to submit it. Previously the only thing standing in the way was SameSite=Lax
on a cookie owned by another module;
- the POST no longer re-validates the client, redirect and scope, which
deletes a second copy of the GET's logic.
For consent phishing: registration is open and `client_name` is unverified, so
anyone can register as "Open Library". The redirect host is the one claim an
impostor cannot fake — a lookalike must send the patron somewhere it controls —
so the consent screen now shows it, alongside a plain statement that the
application registered itself and has not been verified (RFC 7591 §5).
The mock consumer now reads the handle out of the rendered page, which is what
a real patron's browser does, so it demonstrates that a consumer *cannot* mint
its own approval.
398 passed on Postgres, 5 Playwright, mock consumer green end to end.
Refs #209
Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
docs/OAUTH2.md covers the endpoints, scopes, the flow, the federation story, and every security property with the reason it exists — each one pinned by a `test_attack_*` test, so the doc and the suite say the same thing. docs/OL_BORROW_HANDOFF.md argued a design that was discarded in the discussion it came from (Open Library forwarding the patron's IA S3 keys to Lenny). It is now a pointer to #209 and OAUTH2.md rather than a deletion, so an old link does not silently lead nowhere — or worse, lead someone to implement it. Marks all four milestones complete in the plan and replaces the open questions with the five that actually need Mek's call, including two surfaced by review: client verification on the consent screen, and the absence of any way to disable an abusive client without hand-written SQL. Refs #209 Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
Two stages, because Open Library requires Python 3.14 and Lenny runs 3.12 — they cannot share an interpreter, which is honest given they are two services. Stage 1 runs inside an Open Library checkout and harvests the LIVE lennyforlibraries.org feed with OL's real parser. Stage 2 takes that output and drives a local Lenny node's real OAuth 2.0 server: discover, register, consent, exchange, borrow, read loans back. Verified today against the live feed: 94 import records from 95 publications, 69 carrying a borrow acquisition registered with no human coordination (RFC 7591) consent -> code -> back-channel exchange -> access + refresh token borrowed edition 46539165, due date returned loans read back through the API That last step is the thing Open Library could not do at all before: it had no way to answer "what does this patron have on loan?" without either holding a credential it should not, or a key that reads every patron. Stage 1 requires openlibrary#13561; without it the harvest finds zero borrowable publications and the arc has nothing to borrow. Refs #209 Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
Two of the plan's open questions had answers clear enough not to wait on, and
both matter because registration is open.
`OAuthClient.disable()` — there was no way to stop an abusive client without
hand-written SQL, and deletion is blocked anyway by the codes referencing it.
Disabling hides the client from `OAuthClient.get`, so authorize, token and
revoke all refuse it without needing their own check, and it revokes the tokens
the client already holds. Blocking new tokens alone would have left it working
for up to an hour, and refreshing for ninety days.
`sweep_expired()` — codes and tokens accumulated forever while an
unauthenticated registration endpoint let anyone add to the pile. Nothing calls
it automatically; it is there for a cron or a console.
Two details in the sweep are deliberate and easy to get wrong:
- it keeps recently-expired codes, because reuse detection reads a *spent*
code to revoke the tokens it produced. Deleting on expiry would turn a
detected replay into a plain "invalid code".
- it keeps a token row until its *refresh* token is dead, not its access
token. Sweeping on access expiry would break every legitimate refresh.
Also applies three index findings from review: drops four explicit indexes that
duplicated the UNIQUE constraints Postgres already indexes, and adds one on
`authorization_code_id`, which `revoke_for_code()` uses on the reuse-detection
path and was scanning the whole token table.
402 passed on Postgres, 400 on SQLite. Migration verified with a down/up
round-trip against real Postgres.
Refs #209
Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
Adversarial review round 2 went after the endpoints, which round 1 had only glanced at. It found one critical bug and several spec violations. CRITICAL — /oauth2/borrow bypassed every lending rule. It called Loan.create directly instead of Item.borrow, which is the only place lending policy lives, and so skipped all four of its guarantees: open-access items are not lendable, the per-patron concurrent limit, the per-item copy count, and the SELECT ... FOR UPDATE that makes the last two safe under concurrency. Verified before fixing: an open-access item lent (201), a limit of 2 producing 5 loans, and a single copy lent to two different patrons at once — which also drives available_copies negative, so the OPDS feed would have lied about availability too. That is a controlled-digital-lending violation reachable by any client a patron grants `borrow` to. It survived because no test covered a *successful* borrow at all: the only borrow tests used a nonexistent edition or stopped at a scope refusal. Item.borrow now takes `hashed=True`, mirroring Loan.exists/Loan.create, since the token holds only the hash. HIGH — the consent handle was replayable. It carried no id, so one consent click could mint an unbounded number of grants for ten minutes, and clicking "Not now" invalidated nothing: replaying the same handle with decision=allow issued a code anyway. Handles now carry a random id that is spent on use, whichever way the patron decided. The store is per-process, which is honest rather than ideal — noted in the code and on #209. HIGH — the app-wide CORS policy (`allow_origin_regex=".*"` with allow_credentials) reflected any origin on the consent screen, a cookie- authenticated page rendering that handle. Only SameSite=Lax stood in the way, and the OAuth code does not own that cookie. The consent endpoint now refuses cross-origin reads outright, from a middleware layered outside CORSMiddleware so it actually wins. Also: X-Frame-Options/frame-ancestors on the consent screen (RFC 6749 §10.13); `iss` on the authorization response and in the metadata (RFC 9207 — the mix-up defence, which matters precisely because this design assumes many nodes); client_id_issued_at/client_secret_expires_at (RFC 7591 §3.2.1, required); WWW-Authenticate on invalid_client (RFC 6749 §5.2); and registration now refuses redirect URIs with a fragment (RFC 6749 §3.1.2 — the code would land after the '#' and never reach the client) or control characters. Test hygiene: the fixture now clears `loans` too. Lending policy counts a patron's active loans, so one left behind changed what the next test was allowed to do — the borrow tests passed alone and failed in a group. The borrow-policy tests are marked Postgres-only, because SQLite neither autoincrements the BigInteger keys nor implements SELECT FOR UPDATE, so "passing" there would mean nothing. 416 passed on Postgres, 410 on SQLite, 5 Playwright, mock consumer green. Refs #209 Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
RFC 8414 requires the metadata document at the origin root, and there is no catch-all `location /` in lenny.conf — so nginx returned 404 for it. The whole "a consumer given only a base URL can discover this node" story was unreachable in the shipped Docker deployment, however well it worked against the app. The tests did not catch this because they drive the ASGI app directly and never touch nginx. Found by adversarial review reading the deployment rather than the code. Config verified with `nginx -t` against the real conf.d (syntax ok; the remaining complaint is a limit_req_zone defined outside conf.d and absent in a bare container). Refs #209 Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
Two things: the design question Mek settled, and the last outstanding round-2 finding. LENNY_OAUTH2_ALLOWED_CLIENT_HOSTS restricts who may register a client. Empty means open, which is the default and the property the many-nodes design rests on — a consumer has to be able to register against a node nobody told it about. An operator who wants a closed node names the hosts. Every redirect_uri on a registration is checked, not just one: otherwise a client registers a legitimate host alongside one an attacker controls and picks the second at authorization time. Loopback stays permitted so a closed node is still developable against. The race (round 2, H5): the atomic claim is correct, but the winner's token is created *after* it commits. A loser arriving in that window detected reuse and called revoke_for_code — which swept nothing, because the winner's token did not exist yet — and the winner's token was then created live. So the protection that reuse detection exists to give was absent exactly when two requests overlap, which is the case it is for. Realistic on the refresh path, which needs no PKCE verifier. Fixed by recording the revocation on the *grant* rather than only on the tokens that happen to exist at that moment: `AuthorizationCode.grant_revoked_at`, set by revoke_for_code and checked by AccessToken.issue, so a token issued after the replay was seen is born revoked. Ordering stops deciding the outcome, which is the property that was missing — the alternative was restructuring transaction ownership across three methods. Both are covered by tests that fail against the previous code. 423 passed on Postgres, 417 on SQLite. Migration verified with a down/up round-trip. Refs #209 Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
mekarpeles
force-pushed
the
oauth/authorization-server
branch
from
September 6, 2026 03:59
66ebdbe to
24714b3
Compare
From mining PR #158 as a reference. Its token model is worse than this one and was not taken (see below), but its operator ergonomics and deployment plumbing found four genuine gaps. The big one: this server refused every native app. `_acceptable_redirect` accepted only https and loopback, and registration is the only way to create a client — so a reading app could not register at all. Production Lenny already hands native OPDS readers `opds://authorize/` through the older flow, so the new server was locking out exactly the clients Lenny exists to serve. Now accepts private-use URI schemes per RFC 8252 §7.1: `opds://`, the convention this deployment already speaks, and reverse-DNS like `com.example.reader://`. A single-label scheme such as `myapp://` is still refused — any other app on the device can claim it, which is the attack §7.1 is written against. Also honours `token_endpoint_auth_method: "none"` (RFC 7591), so a native app registers as a public client instead of being handed a secret it cannot keep and would ship in its app bundle. PKCE is already mandatory, which is what makes a secretless client safe. And since a browser will not reliably follow a private-use scheme, the authorization step renders a handoff page rather than issuing a 303 the browser may drop. Operator commands: `scripts/oauth2_client.py` with list/disable/sweep, wired to `make oauth2-clients`, `make oauth2-disable CLIENT=...`, `make oauth2-sweep`. Registration is open by default, so "stop this client" needed to be something other than an ORM call in a Python console — and client ids are server-generated, so `list` is the only way to find one. Rate limits: the new endpoints were falling through to the general 30/min api zone while a tighter `oauth` zone already existed for the OPDS auth routes. /register is unauthenticated and every call adds a row — at the api rate one address could add ~43k client rows a day. Verified with `nginx -t`. `LENNY_OAUTH2_ALLOWED_CLIENT_HOSTS` is now in configure.sh alongside its empty-means-open sibling, so it exists in every generated .env. It was documented but unplumbed, and .env is chmod 600 and only written when absent — there was no discovery path. Deliberately NOT taken from #158: JWT access tokens (no revocation, raw patron email in every Authorization header, signed with the same LENNY_SEED as session cookies); its cleanup task (deletes spent codes immediately, destroying the evidence reuse detection needs, and runs in every worker); its manual X-Forwarded-For parsing; and its JWT-as-session-cookie fallback, which would let a scoped access token authenticate as a full patron session. 432 passed on Postgres, 426 on SQLite. Refs #209, #158 Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
Member
Author
Mined #158 — it found a real gapThe headline: this server refused every native app, and I'd have shipped it that way.
Now accepts private-use URI schemes per RFC 8252 §7.1: Two things fell out of that:
Also taken from #158
Deliberately not taken
Since the last updateAlso landed: the registration allowlist you specified (empty = open; every 432 passed on Postgres, 426 on SQLite. Rebased onto main after #212. One item still needs your callThe metadata issuer follows the |
The open question was whether to require LENNY_PROXY or add a host allowlist. Neither: Lenny already has a single source of truth for its own public URL, and the OAuth metadata had no business inventing a second one. `LennyAPI.make_url` builds every absolute link in the OPDS feed and the Authentication Document. Production proves it works — lennyforlibraries.org emits https://lennyforlibraries.org/... for both. If it were wrong the feed would already be broken and an operator would have noticed. So the metadata now uses it unconditionally and the request-derived fallback is gone. Why that fallback was a real problem, not a style preference: RFC 8414 §3.3 makes the issuer security-relevant — a consumer compares it against the URL it fetched and refuses on mismatch — so deriving it from the Host header let anyone who could set that header, or seed a path-keyed cache, advertise an attacker-controlled token_endpoint for a lax client to POST its client_secret and authorization code to. A test now asserts a hostile Host header cannot move any advertised endpoint. The cost is real and worth stating: a node reached at an address its config does not know about now advertises endpoints nobody can reach. That is why it warns on every such request, naming the variable to set. Discovering this silently is exactly how the forwarded-IP default survived from #201 to #210. Found while verifying: the mock consumer stopped working locally, because it follows discovery honestly and was being sent to localhost:8080. That is the change behaving correctly — running on a non-default port now needs LENNY_PROXY. Documented in docs/OAUTH2.md and tests/e2e/README.md, and re-verified end to end with it set. Also pins that `iss` on the authorization response equals the advertised issuer; otherwise RFC 9207's mix-up defence compares two different things. 434 passed on Postgres, 428 on SQLite. Refs #209 Claude-Session: https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas
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
1 participant
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.
Draft. Implements #209. Opening early so the design can be argued before more is built on it.
Why
Lenny stores "logged in" as a cookie on its own domain. A consumer — Open Library, another catalogue, a reading app — cannot POST to Lenny and receive a usable session:
Set-Cookielands on the consumer's HTTP client, not the patron's browser, and the cookie isDomain-scoped,HttpOnly,SameSite=Lax. So a consumer has no way to answer "what does this patron have on loan?"Alternatives considered and rejected:
Authorization Code + PKCE avoids all of them: every token is per-patron, granted by that patron, and never travels through the browser.
Designed for many nodes
The assumption is that many organisations run Lenny nodes and a consumer must interop with all of them. Manual client provisioning per node does not scale, so this includes:
/.well-known/oauth-authorization-server— a consumer given only a base URL can discover the endpointsThis is the Mastodon topology: many independent authorization servers, clients that must work with all of them and have no prior relationship with any.
What is here
lenny/core/oauth2.py— clients, authorization codes, access/refresh tokenslenny/routes/oauth2.py— authorize, token, revoke, register, loans, borrowalembic a7c4e91d2f80— three tables; every secret stored as a SHA-256 digest, so a DB dump yields nothing replayableoauth2_consent.html— patron-facing consent screenscripts/mock_openlibrary.py— a stand-in consumer that drives the whole flowSecurity properties the implementation intends to hold, each to be pinned by a test:
redirect_uriand PKCE challengeplainrefusedredirect_uriis an error shown to the patron, never a redirect (otherwise this endpoint becomes an open redirector)/authorizeStatus
Scaffolding. Not ready for review yet. Tests lead the remaining work.
docs/OAUTH2.mdPlan and deliverables:
docs/plans/oauth2-authorization-server.md.Existing
/oauth/*OPDS routes are untouched — this is additive, not a migration.https://claude.ai/code/session_01UViYm1nKqJkiKJWJUr1eas