fix: allow opting in to insecure OAuth requests

[jeswr]

Problem

oauth4webapi v3 enforces HTTPS on every request, so discoveryRequest / dynamicClientRegistrationRequest / authorizationCodeGrantRequest all throw only requests to HTTPS are allowed against an http: issuer. That blocks the auth-code + PKCE + DPoP flow against a local Community Solid Server on http://localhost:3000.

Change

Following @langsamu's review, this no longer tries to detect loopback issuers. A global switch is exposed instead and the caller decides:

import { InsecureConfiguration } from "@solid/reactive-authentication"

InsecureConfiguration.allow()

allow() is deprecated on purpose, so the security implication shows up at the call site rather than only in docs, and it logs to console.error when called. Enforcement stays on by default — the library never relaxes it on the consumer's behalf.

Applied across all three providers:

• DPoPTokenProvider — the original bug; had no insecure handling at all.
• BearerTokenProvider — replaces const oauthAllowInsecureRequests = true, which unconditionally disabled HTTPS enforcement for every consumer, including production. Resolves its // TODO: Configure properly for insecure localhost only.
• ClientCredentialsTokenProvider — had no insecure handling either, so it could not reach a local issuer at all.

index.html calls InsecureConfiguration.allow(), since the demo targets a local CSS.

Notes for review

• Behaviour change for BearerTokenProvider: insecure requests were previously always on and are now opt-in. Anyone relying on the old implicit behaviour against a local server needs the allow() call. That is the point of the change, but it is a break worth calling out.
• Service workers are a separate realm. A static flag set on the page does not reach reactive-fetch-worker.js, so ReactiveFetchWorkerManager consumers currently have no way to opt in. Plumbing config through the worker's message channel is a bigger change — happy to do it here or in a follow-up, whichever you prefer.
• The loopback predicate and its node:test suite are both removed, along with the test script (the tested function no longer exists, and feat: per-issuer session cache — stop re-running the full auth flow (popup included) on every 401 #11 is separately introducing vitest — leaving package.json untouched here avoids fighting over the test script).

Rebased onto current main. npm run build (tsc) clean.

[Copilot]

Pull request overview

This PR fixes local-development logins for the auth-code + PKCE + DPoP flow by relaxing oauth4webapi’s HTTPS-only enforcement only when the OIDC issuer is a loopback http: origin (e.g. http://localhost:3000), aligning behavior with the existing Bearer flow’s ability to permit insecure requests for dev.

Changes:

• Added an isLoopbackHttpIssuer guard and threaded oauth.allowInsecureRequests into DPoP discovery, dynamic client registration, and token exchange calls.
• Added node:test regression tests for the loopback-issuer predicate.
• Added an npm test script to run the new TypeScript tests via Node’s test runner.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/DPoPTokenProvider.ts	Adds loopback HTTP issuer detection and passes allowInsecureRequests options through oauth4webapi calls for DPoP flow.
test/DPoPTokenProvider.test.ts	Adds regression tests validating when HTTP loopback issuers should/shouldn’t be allowed.
package.json	Adds a test script to run the new node:test suite.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jeswr]

Resolved by removing the predicate entirely. Per @langsamu's review the PR no longer tries to work out what is loopback — InsecureConfiguration.allow() is now an explicit global opt-in, so there is no hostname parsing left to bypass.

[jeswr]

No longer applicable — the predicate it would have guarded is gone. There is no URL parsing left to regression-test; the remaining surface is a single boolean flag.

[langsamu]

I have run into this annoyance myself several times when running CSS locally.

But I would much rather not get into the heuristic of determining what is loopback. Also I would rather not relax the draconian behavior of the underlying library.

How about another design where there is a global switch that library consumers can toggle to enable insecure requestsm like

class InsecureConfiguration {
    private static allowed: boolean = false

    /** @deprecated */
    static allow(){
        console.error("Insecure requests allowed for oauth4webapi")
        this.allowed = true
    }

    static get requestOptions(): { [oauth.allowInsecureRequests]?: boolean } {
        return {[oauth.allowInsecureRequests]: this.allowed}
    }
}

which could be used like

const tokenResponse = await oauth.authorizationCodeGrantRequest(
  as,
  client,
  auth,
  params,
  callback,
  verifier,
  {
    DPoP: dpop,
    ...InsecureConfiguration.requestOptions
  }
)

We leave it to the caller to make the decision, help them with deprecation warning and error in console, don't interfere with their security stance outside their knowledge and don't get into URI parsing.

[langsamu]

Like 127.com?

[jeswr]

Yes — 127.com would have matched, as would 127.0.0.1.evil.com. Both are gone: the whole predicate is removed in favour of your InsecureConfiguration design.

[langsamu]

I think URL normalizes these to the above.

[jeswr]

Correct, URL normalises ::1 to [::1], so that branch was dead code. Moot now that the predicate is deleted.

[langsamu]

What about 127.0.0.2? (Since the block is 127.0.0.0/8.)

[jeswr]

Right, 127.0.0.2 is in-block and the exact-match arm missed it — which is what pushed me to the sloppy startsWith("127."). Removed along with the rest of the heuristic.

[langsamu]

Private utility should not be exported.

[jeswr]

Fixed — nothing extra is exported now. isLoopbackHttpIssuer is deleted; the only new export is the InsecureConfiguration class itself, which is deliberately public API.

[langsamu]

This is a private utility method. I'm not sure it should be documented like this.

[jeswr]

Agreed. The oversized doc block is gone with the function. The new class carries a short block in the style of the rest of the codebase.

[jeswr]

Rebased fix/loopback-issuer onto current main (was CONFLICTING → now MERGEABLE). Main had since refactored DPoPTokenProvider — a string callbackUri constructor field replacing the old #getCallback, plus {signal: request.signal} threaded through every oauth.* call. I integrated those changes and re-threaded the loopback fix on top: the insecureOption (only non-empty for loopback http: issuers) is now spread alongside signal into discoveryRequest, dynamicClientRegistrationRequest, and authorizationCodeGrantRequest, and the registration/token requests use the new this.#callbackUri field. Production issuers stay HTTPS-strict; only loopback http: (localhost / *.localhost / 127.0.0.0/8 / ::1) is relaxed, mirroring how BearerTokenProvider threads oauth.allowInsecureRequests. npm run build (tsc) and npm test (the new node:test predicate regression — 3/3) both green.

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

[jeswr]

@langsamu adopted your design — thanks, it is clearly the better call. No URI parsing, no heuristics, and the library no longer overrides anyone's security stance implicitly.

Implemented as src/InsecureConfiguration.ts essentially as you sketched it, with static #allowed to match the #-private field convention used elsewhere (Mutex, the providers). Threaded through all three providers.

Two things I went slightly beyond the original scope on, both easy to drop:

1. BearerTokenProvider had const oauthAllowInsecureRequests = true with a // TODO: Configure properly for insecure localhost only. That unconditionally disabled HTTPS enforcement for every consumer, so the switch resolves the TODO. It does mean insecure requests are now opt-in there rather than always on.
2. ClientCredentialsTokenProvider had no insecure handling at all, so a "global" switch would otherwise have missed one of three providers.

One gap worth your view: a static flag is per-realm, so a page calling allow() does not reach reactive-fetch-worker.js. ReactiveFetchWorkerManager consumers therefore have no way to opt in today. Plumbing it through the worker's message channel is a real design decision, so I left it out — say the word and I will add it here or open a follow-up.

Also dropped the loopback predicate's node:test suite and the test script, since the code under test is gone and #11 is separately introducing vitest.