jeswr / jeswr/auth-solid

Sign-off: security-review + external-maintainer-readiness (branch sec-review/maintainer-readiness)

Open
#2 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
0
Forks
0
PR merge metrics
No merged PRs in 30d

Description

🤖 PSS agent — @jeswr's agent for \`prod-solid-server\` / the Solid app+Pod-Manager suite

Branch \`sec-review/maintainer-readiness\` (HEAD \`ecc3d22\`) is a security-review + external-maintainer-readiness pass on \`@jeswr/auth-solid\` (bead suite-tracker-68b0). It **changes security behaviour** (strictly hardening), so it stages here for your sign-off rather than direct-landing.

## Security findings fixed (all with regression tests)

**HIGH — ID-token JWS signature was never verified.** In the installed \`@auth/core\` 0.41 flow, \`processAuthorizationCodeResponse\` validates alg/claim-presence/iss/aud/time but **not** the JWS signature (that lives only in oauth4webapi's separate \`validateApplicationLevelSignature\`, which @auth/core never calls). So \`profile.webid\` was trusted from a signature-unverified token. Fix: new \`src/idTokenVerify.ts\` verifies the raw id_token against the OP JWKS (jose \`jwtVerify\`, issuer+audience pinned, rejects HS*/none pre-JWKS, fail-closed); WebID now read from the signature-verified payload. Tests: correctly-signed accepted; wrong-key / tampered / HS256 / no-id_token / wrong-aud / wrong-iss / sub-mismatch all rejected.

**HIGH — discovery/JWKS fetch followed redirects.** An https metadata/JWKS endpoint that 302s to http (or a foreign origin) would be followed inside fetch, allowing plaintext/cross-origin JWKS key substitution that defeats the signature boundary above. Fix: \`fetchNoRedirect\` (redirect:manual + reject 3xx/opaque-redirect, fail-closed) on both discovery and JWKS, layered on the existing https-only transport guard. Tests: discovery/JWKS 302 rejected, opaque-redirect rejected, happy path still verifies.

**MEDIUM — every customFetch leg now https-enforced** (was token-leg only; @auth/core sets oauth4webapi \`allowInsecureRequests\` whenever a customFetch exists, so this package is the *only* https enforcement — an http userinfo_endpoint would have received the access token in clear).

**MEDIUM — empty \`DPoP-Nonce\` header no longer triggers a spurious retry** (Headers.get returns "" not null; the gate now treats falsy as absent).

**MEDIUM — api-extractor pinned** as a devDependency (was \`npx --yes @microsoft/api-extractor@7\` = non-reproducible / network-dependent gate).

## Verification
- roborev PASS on HEAD (single daemon, serialized); all prior-round findings walked back and confirmed gone from HEAD (not just a diff-scoped later PASS).
- I re-ran the security suite myself on HEAD: **102 tests pass** including the 9 signature-verification cases and the 4 redirect-refusal cases.
- Clean-room \`GIT_SSH_COMMAND=false npm ci --ignore-scripts\` OK; dist rebuilt + committed; api-extractor snapshot byte-identical (all new code is internal); check:dist + api:check green; #78 lockfile clean.
- One behaviour change to note for consumers: the \`profile\` callback is now **async** (it awaits JWKS verification).

## Follow-ups (beaded, non-blocking)
- Refresh-grant helper (persists refreshToken/expiresAt but ships none; compose with @jeswr/solid-dpop).
- Wire @jeswr/guarded-fetch into the \`Solid({ fetch })\` seam (guarded-fetch phase-2) — would consolidate the redirect-refusal helper.
- Suite-wide \`npm ci\`-broken-lockfile sweep (npm-11 drops @emnapi wasm optionals) — a general finding, not specific to this repo.

Merge when you're happy with the security review.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the sec-review/maintainer-readiness branch at HEAD ecc3d22 and read src/idTokenVerify.ts, then run the security suite covering signature verification and redirect refusal. The work is presented as completed and awaiting sign-off, with verification already reported, so there is no unclaimed implementation path for a newcomer.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
authentication, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
15/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.