Merging in os template defaults. Adding workflows and initial docs. by heskew · Pull Request #4 · HarperFast/oauth

heskew · 2025-10-28T23:26:26Z

No description provided.

kriszyp

This looks amazing, excellent docs.
I think the main thing I think might be missing, is showing how this interacts with retrieval of the current user, for request.user, by defining a server.getUser function. All the default permission checks on table access check request.user (or more specifically request.user.role.permission), as well as plenty of our examples of how to check user access, so it seems like showing how to return users with table access might be useful.
Again, great work, this is a fantastic plugin.

kriszyp · 2025-10-29T12:17:19Z

+});
+```
+
+**Note:** Must be called before the OAuth plugin initializes (typically at module load time).


Is this necessary? From reading of the code, it looks like it would handle out of order loading, but maybe I am wrong.

I don't want it to be. lol. Let me check...

verified that registration should be flexible and updated the doc to clarify. thanks for bringing that up!

kriszyp · 2025-10-29T12:17:32Z

+export class MyResource extends tables.Resource {
+	async get(target, request) {
+		// Check if user is authenticated
+		if (!request.session?.oauthUser) {


You can't check request.user?

could if the app, or another plugin, had created a user. this plugin just currently deals with the session. added an example though since that will probably be common.

kriszyp · 2025-10-29T12:20:15Z

+
+```yaml
+http:
+  session:


We support this in the root harperdb-config.yaml? Is this part of what should go in core?

No... we don't have secure session ability yet...

Does this mean using secure cookies? We always use Secure cookies with https, I believe. Or does this mean supporting disabling that?

That was referring to a different concept that got mixed in. Something more long the lines of secure stateless cookies (i.e. encrypted session cookies). So not relevant to harper. I'd see having a jwt for the session cookie data instead of a guid before we go down that road.

kriszyp · 2025-10-29T12:30:17Z

+registerHooks({
+	onLogin: async (oauthUser, tokenResponse, session, request, provider) => {
+		// Create user in your database
+		const user = await tables.User.put({


This occurs on every login, so devs might want to only put the user if it doesn't exist or doesn't match the email/name? I guess you show this in more detail below, so maybe this is fine.

good call out. this could result in some poor copy/paste. updated.

kriszyp · 2025-10-29T12:34:05Z

+	}
+
+	// Find or create user
+	let user = await User.findOne({ email: oauthUser.email }, context);


We don't have a findOne method here, right?

not even close. still working on the docs, making sure it's all good and actual useful (and accurate).

…ion timing

kriszyp

This looks great. I'd still like to see some examples of using server.getUser and request.user, but not necessary for this PR.

heskew · 2025-10-29T23:07:48Z

This looks great. I'd still like to see some examples of using server.getUser and request.user, but not necessary for this PR.

@kriszyp I want more real examples too. :) What would you think about getting this published now. I want to wire it up to a small fabric app - a light blog or something. One of the standard web app examples.

kriszyp · 2025-10-29T23:33:40Z

Go for it! Publish! Love it!

…flow consent cookie, DNS bounding, config hardening Second external-review batch on top of the consent-binding/SSRF work. Consent cookie (Codex #1 sibling-injection High, #7 concurrent-flows Medium): - Switch the consent cookie to a per-flow __Host--prefixed name (__Host-mcp_consent_<flowId>). __Host- forbids a Domain attribute, so a sibling origin can no longer plant a parent-domain cookie to forge the binding (SameSite=Lax doesn't stop siblings — they're same-site). The per-flow id (carried in confirm + upstream state) lets parallel tabs run concurrent flows without clobbering each other's binding. Callback ordering (Codex #5 Medium): - Verify the browser binding BEFORE exchangeCodeForToken and the onLogin hook, so a mismatched (self-approved) flow triggers no upstream exchange and no provisioning side-effects. Regression asserts neither runs on mismatch. SSRF (Codex #3 rebind High, #2 threadpool High, #4 special-use Medium): - Pinned-connect: fetch via https.request with a custom lookup that connects to the exact address the gate validated, while keeping the hostname for TLS SNI + cert verification — closes the DNS-rebinding TOCTOU. (No undici dep: undici isn't importable here; https.request also gives no-redirect-follow for free.) - Bound concurrent DNS resolutions with a permit released only when the raw (uncancellable) getaddrinfo settles, so a flood of blackholed-DNS client_ids can't pin the libuv pool; fast-reject when saturated; dedup concurrent resolutions of the same client_id into one fetch. - Classify against the full IANA IPv4/IPv6 special-purpose registries (192.0.2/24, 198.51.100/24, 203.0.113/24, 192.0.0/24, 192.88.99/24, AS112/AMT, and in-2000::/3 Teredo/ORCHID/documentation), table-driven. Config (Codex #6 Medium): - Normalize the mcp block at load: coerce documented boolean strings (env-expanded "false" no longer leaves a security switch truthy) and require allowedHosts to be an array of exact lowercased hostnames (a scalar is wrapped, not substring-matched; non-strings rejected). Deferred (agreed): the pinnedHttpsFetch path is production-only (tests stub the fetch seam and exercise SSRF via the DNS seam); its lookup is a thin pass-through of the pre-validated addresses. Tests 938/936 pass (2 pre-existing skips); docs updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…sent interstitial (#166) (#167) * feat: Client ID Metadata Documents (CIMD) with SSRF-guarded resolution and consent interstitial (#166) Implements the MCP authorization spec's CIMD support: when a client_id is an HTTPS URL with a non-root path, the AS fetches it as a JSON metadata document instead of doing a DCR lookup. Core changes: - New `src/lib/mcp/cimd.ts`: `isCimdClientId`, `resolveCimdClient`, `resolveClient`. SSRF guards via DNS pre-flight (all A/AAAA records checked against private/loopback ranges), IP-literal rejection, no-redirect fetch, 5 s timeout, 64 KB cap. Per-process cache with `Cache-Control: max-age`-based TTL clamped to [60 s, 86400 s]; negative-cache on client errors (60 s), not on server errors. - New `src/lib/mcp/clientValidator.ts`: shared validators extracted from dcr.ts (`validateRedirectUri`, `validateStringArray`, `validateGrantTypes`, etc.). - `src/lib/mcp/authorize.ts`: `resolveClient` replaces direct `MCPClientStore` lookup. `escapeHtml` and `buildInterstitialPage` for the CIMD consent page. `handleAuthorize` returns 200 HTML for CIMD clients; 302 for stored/DCR clients. New `handleAuthorizeConfirm` for `POST /oauth/mcp/confirm`: verify + consume one-time token, validate `_confirm` marker, redirect to upstream IdP. - `src/lib/mcp/wellKnown.ts`: advertises `client_id_metadata_document_supported: true` when CIMD is enabled (default on). - `src/lib/mcp/token.ts`: `authenticateClient` uses `resolveClient`; handles `CimdClientError` as `invalid_client`. - `src/lib/mcp/index.ts` + `src/lib/resource.ts`: route `POST /oauth/mcp/confirm` to `handleAuthorizeConfirm`; thread `providers` registry into `handleMCPPost`. - `src/types.ts`: `MCPClientIdMetadataDocumentsConfig`, `MCPConfig.clientIdMetadataDocuments`, `MCPClientRecord._cimd`, `MCPClientMetadata.jwks/jwks_uri`. Tests (50 new passing): - `test/lib/mcp/cimd.test.js`: isCimdClientId shape checks, SSRF DNS gate, allowedHosts policy, document validation, cache TTL, resolveClient routing. - `test/lib/mcp/authorize.test.js`: escapeHtml (XSS vectors), buildInterstitialPage (loopback warning, token binding, XSS), handleAuthorize CIMD path (200 HTML), handleAuthorizeConfirm (valid token → 302, single-use, expired, missing _confirm). - `test/lib/mcp/wellKnown.test.js`: CIMD flag present when enabled, absent when disabled. Docs: CIMD section in mcp-oauth.md, four config rows in configuration.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(cimd): separate document-host policy from redirect-host policy clientIdMetadataDocuments.allowedHosts governs which hosts may SERVE metadata documents; redirect URIs are a different policy and now validate against dynamicClientRegistration.allowedRedirectUriHosts (same rules as DCR clients). A trusted vendor whose document declares redirect targets on another host is no longer wrongly rejected. Regression tests added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cimd): address external security review — consent browser binding, token purpose, SSRF gate, cache and fetch hardening Consent flow (the two flow-level blockers): - Bind the CIMD consent to the approving browser: the interstitial sets an HttpOnly/Secure/SameSite=Lax nonce cookie whose SHA-256 travels inside the confirm token and upstream state; POST /oauth/mcp/confirm and the OAuth callback both require a hash match before proceeding (new src/lib/mcp/consentBinding.ts). A malicious client can no longer self-approve the interstitial and hand the victim the upstream IdP URL. - Enforce token purpose at the callback: a confirm token presented as upstream OAuth state is rejected like an invalid token (it previously passed the mcp/providerName checks and skipped consent entirely). SSRF DNS gate: - IPv4: also reject 0/8, 100.64/10 (CGNAT), 198.18/15, 224/4+; malformed input fails closed. - IPv6: real parser (:: compression, embedded dotted-quad, zone index); allow only global unicast 2000::/3, with v4-mapped addresses classified by their embedded IPv4 address. Closes the ::, non-canonical loopback, and hex-form v4-mapped bypasses. - All gate rejections (didn't resolve / blocked address) return one generic invalid_client message; details are logged server-side only, so callers can't probe the server's internal DNS view. - Reject dot path segments (raw or percent-encoded) and non-lowercase scheme spellings in CIMD client_ids before URL normalization erases them. Fetch path: - Only 200 OK is accepted (404/500 JSON no longer resolves as a client). - One deadline across DNS, connect, headers, and body read — a trickling body can no longer hold connections open past the timeout. - fetchTimeoutMs/maxDocumentBytes are coerced to finite positive numbers; NaN/Infinity/garbage fall back to defaults instead of failing open. Cache: - LRU-bounded to 1000 entries (keys are attacker-chosen input). - Failures are never cached (CIMD draft forbids caching errors/invalid documents); negative caching removed. - Cache-Control no-store/no-cache floor at the 60 s DoS floor. - Cached records revalidate against the live allowedRedirectUriHosts on every hit, so tightening the policy takes effect immediately. Interstitial: - Served with X-Frame-Options: DENY, CSP frame-ancestors 'none', and Cache-Control: no-store (page carries the single-use confirm token). - Displays the authoritative client_id hostname; client_uri is labelled unverified; unparseable redirect_uri degrades instead of throwing. Plus error-cause chaining and server-side logging on swallowed catch paths (confirm verify, token client lookup). 52 new/updated tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cimd): decode IPv4-in-IPv6 transition forms and fail closed on unknown DNS family Follow-up from the cross-model review pass on 195cf21. - isPrivateIpv6 now decodes the IPv4 embedded in 6to4 (2002::/16) and ISATAP and classifies it via isPrivateIpv4, and rejects Teredo (2001:0000::/32) outright. These transition forms sit inside the 2000::/3 global-unicast allow but can target a private IPv4 — previously a 6to4/ISATAP address wrapping 10/8 or 127/8 was allowed. - checkHostSsrf fails closed on any DNS address family other than 4/6 rather than skipping both range checks (defense-in-depth; dns.lookup only returns 4/6 today). - Tests for 6to4/ISATAP/Teredo private targets, a public 6to4 pass-through, and the unknown-family reject. In-flight fetch dedup (thundering herd on an uncached attacker URL) was raised as a suggestion and is deferred to rate limiting (#163), bounded meanwhile by the 64 KB / 5 s caps. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cimd): address second Codex review — pinned-connect, __Host- per-flow consent cookie, DNS bounding, config hardening Second external-review batch on top of the consent-binding/SSRF work. Consent cookie (Codex #1 sibling-injection High, #7 concurrent-flows Medium): - Switch the consent cookie to a per-flow __Host--prefixed name (__Host-mcp_consent_<flowId>). __Host- forbids a Domain attribute, so a sibling origin can no longer plant a parent-domain cookie to forge the binding (SameSite=Lax doesn't stop siblings — they're same-site). The per-flow id (carried in confirm + upstream state) lets parallel tabs run concurrent flows without clobbering each other's binding. Callback ordering (Codex #5 Medium): - Verify the browser binding BEFORE exchangeCodeForToken and the onLogin hook, so a mismatched (self-approved) flow triggers no upstream exchange and no provisioning side-effects. Regression asserts neither runs on mismatch. SSRF (Codex #3 rebind High, #2 threadpool High, #4 special-use Medium): - Pinned-connect: fetch via https.request with a custom lookup that connects to the exact address the gate validated, while keeping the hostname for TLS SNI + cert verification — closes the DNS-rebinding TOCTOU. (No undici dep: undici isn't importable here; https.request also gives no-redirect-follow for free.) - Bound concurrent DNS resolutions with a permit released only when the raw (uncancellable) getaddrinfo settles, so a flood of blackholed-DNS client_ids can't pin the libuv pool; fast-reject when saturated; dedup concurrent resolutions of the same client_id into one fetch. - Classify against the full IANA IPv4/IPv6 special-purpose registries (192.0.2/24, 198.51.100/24, 203.0.113/24, 192.0.0/24, 192.88.99/24, AS112/AMT, and in-2000::/3 Teredo/ORCHID/documentation), table-driven. Config (Codex #6 Medium): - Normalize the mcp block at load: coerce documented boolean strings (env-expanded "false" no longer leaves a security switch truthy) and require allowedHosts to be an array of exact lowercased hostnames (a scalar is wrapped, not substring-matched; non-strings rejected). Deferred (agreed): the pinnedHttpsFetch path is production-only (tests stub the fetch seam and exercise SSRF via the DNS seam); its lookup is a thin pass-through of the pre-validated addresses. Tests 938/936 pass (2 pre-existing skips); docs updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(cimd): drop redundant Buffer.from in Buffer.concat Buffer.concat accepts Uint8Array[] directly (gemini review nit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cimd): reject query strings in CIMD client_ids The draft (§3) says client_id URLs SHOULD NOT include a query string; enforce it as part of the strict URL-shape profile — a dynamic server could otherwise mint unlimited exact-match client_id aliases of one document by echoing query variants. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73 * fix(cimd): tear down rejected connections, bound total resolution concurrency Third external review batch: - Abort the pinned connection when a response is rejected after headers (non-200, non-JSON, oversize) — the deadline timer is cleared on exit, so a hostile endpoint could otherwise hold rejected sockets open indefinitely. - Bound TOTAL concurrent CIMD resolutions (DNS + connect + body) at 16, fast-rejecting past the cap; the DNS permit alone released too early to stop unique client_ids fanning out into unbounded HTTPS work. - Drop MAX_CONCURRENT_DNS from 8 to 2 — below the default 4-thread libuv pool, so blackholed lookups can never starve fs/crypto users. - Remove the jwks/jwks_uri carry-through: no consumer or validation until #159, which will add the plumbing alongside both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73 * chore(cimd): defense-in-depth minors from external agent review - Add 224/4 multicast to the IPv4 special-use table so the table alone is complete (the a >= 224 early return remains as a fast path). - Document that CIMD interactive authorization requires HTTPS — the __Host-/Secure consent cookie is silently dropped on plain-HTTP origins. - Note readConsentNonce's base64url dependency and first-match semantics for future readers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73 * docs(cimd): spell out 224/4 multicast + 240/4 reserved in the SSRF header Replace the cryptic 224/4+ shorthand in the module-header range list (gemini nit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73 --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

Merging in os template defaults

4a90da1

heskew marked this pull request as draft October 28, 2025 23:31

heskew changed the title ~~Merging in os template defaults~~ Merging in os template defaults and adding workflows Oct 28, 2025

heskew added 2 commits October 28, 2025 16:38

Init a couple workflows

e0ed8ab

Adding some initial docs

36d32de

heskew changed the title ~~Merging in os template defaults and adding workflows~~ Merging in os template defaults. Adding workflows and initial docs. Oct 29, 2025

heskew added 2 commits October 28, 2025 18:50

formatting fix

36de385

lint fix

12b2999

heskew force-pushed the open branch 3 times, most recently from 60552ba to 8b7f61b Compare October 29, 2025 02:36

workflow update and adding codeowners

d441e83

heskew force-pushed the open branch from 8b7f61b to d441e83 Compare October 29, 2025 02:38

heskew marked this pull request as ready for review October 29, 2025 02:39

heskew requested review from Ethan-Arrowood and kriszyp October 29, 2025 02:39

kriszyp reviewed Oct 29, 2025

View reviewed changes

heskew added 3 commits October 29, 2025 11:03

doc updates for correctness

720ba71

Update hook registration notes to deemphasize importance of registrat…

c2f2f2b

…ion timing

improve simple getting-started onLogin hook example

78fa4aa

kriszyp approved these changes Oct 29, 2025

View reviewed changes

heskew added 2 commits October 29, 2025 14:10

since someone will ask why 20 was left out

2cf442e

Another auth verification example

cad8a9d

heskew merged commit 124b59f into main Oct 29, 2025
3 checks passed

heskew deleted the open branch October 30, 2025 15:38

heskew mentioned this pull request Apr 24, 2026

ci(claude): address external review findings (mirror of harper #402) #65

Merged

Uh oh!

Conversation

heskew commented Oct 28, 2025

Uh oh!

kriszyp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kriszyp left a comment

Choose a reason for hiding this comment

Uh oh!

heskew commented Oct 29, 2025

Uh oh!

kriszyp commented Oct 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants