Date: Thu, 24 Sep 2026 17:26:11 +0200
Subject: [PATCH 4/7] Refactor application code and remove obsolete logic
---
projects/opds-pkce-test-server/README.md | 92 +--
projects/opds-pkce-test-server/server.mjs | 524 +++++-------------
.../opds-pkce-test-server/server.test.mjs | 165 +++---
src/main/network/http.ts | 7 +-
src/main/network/opdsPkce.ts | 158 ++----
src/main/redux/sagas/auth.ts | 112 +---
test/main/network/opdsPkce.test.ts | 207 +++----
7 files changed, 339 insertions(+), 926 deletions(-)
diff --git a/projects/opds-pkce-test-server/README.md b/projects/opds-pkce-test-server/README.md
index a224dc0ecd..e90f8e7941 100644
--- a/projects/opds-pkce-test-server/README.md
+++ b/projects/opds-pkce-test-server/README.md
@@ -1,100 +1,38 @@
-# OPDS Authorization Code + PKCE Test Server
+# OPDS Authorization Code with PKCE test server
-This is a dependency-free, local-only development server for testing an OPDS
-client's OAuth 2.0 Authorization Code flow with PKCE. It is not intended for
-production use and does not authenticate real users.
+This dependency-free loopback server exercises the flow proposed in
+[opds-community/drafts#100](https://github.com/opds-community/drafts/issues/100).
+It is for local development only and does not authenticate real users.
-The current OPDS Authentication 1.0 draft does not define Authorization Code
-+ PKCE. This server uses the following proposed extension type for integration
-testing:
-
-```text
-http://opds-spec.org/auth/oauth/authorization-code-pkce
-```
-
-The token endpoint uses the local integration link relation:
-
-```text
-token
-```
-
-The authentication type and the `token` relation are not part of the published
-OPDS Authentication 1.0 draft. If the OPDS community adopts a different
-discovery contract, update the server relation and the client's `LINK_TYPE`
-mapping together.
-
-The client validates the advertised OAuth authorization-server metadata before
-starting authorization. The metadata issuer must match the OPDS authentication
-document, both endpoint links must match the metadata, `S256` and authorization
-response issuer identification must be supported, and callbacks must contain a
-matching `iss` value.
-
-## Run
-
-From the repository root:
+Run it from the repository root:
```powershell
npm run start:opds-pkce-test-server
```
-The server listens only on `127.0.0.1`. Open:
-
-```text
-http://127.0.0.1:49152/
-```
-
-Add the protected catalog to Thorium:
+Then add this protected catalog to Thorium:
```text
http://127.0.0.1:49152/opds/v2/catalog
```
-When the authorization page opens in the system browser, select **Authorize**.
-The default callback URI is `opds://authorize/`.
+The authentication document contains only the proposed flow type and its
+required `authenticate` and `refresh` links. The server expects the shared OPDS
+client ID `http://opds-spec.org/auth/client`, the callback `opds://authorize/`,
+and PKCE `S256`. The `refresh` link targets `/token`, which handles both the
+authorization-code exchange and refresh-token grant.
-## Configuration
+Because this local server uses plain HTTP, only development and CI builds of
+Thorium accept its loopback endpoints. Production builds require HTTPS.
-The optional first command-line argument changes the port:
+Use a different port with an optional argument:
```powershell
node projects\opds-pkce-test-server\server.mjs 49153
```
-The direct-run server also supports:
-
-```text
-OPDS_PKCE_PORT
-OPDS_PKCE_CLIENT_ID
-OPDS_PKCE_REDIRECT_URI
-```
-
-For browser-only inspection, set `OPDS_PKCE_REDIRECT_URI` to
-`http://127.0.0.1:49152/callback`. For Thorium integration, leave the default
-`opds://authorize/` URI.
-
-## Endpoints
-
-| Endpoint | Purpose |
-| --- | --- |
-| `/auth` | Public OPDS Authentication Document |
-| `/.well-known/oauth-authorization-server` | OAuth Authorization Server Metadata |
-| `/authorize` | Authorization UI and code issuance |
-| `/token` | Authorization-code exchange and refresh-token grant |
-| `/opds/v2/catalog` | Protected OPDS 2 feed |
-| `/publication.txt` | Protected acquisition test resource |
-| `/health` | Health status and in-memory object counts |
-
-Authorization codes are single-use and expire after two minutes. The token
-endpoint requires PKCE `S256`, rejects a non-empty `client_secret`, and removes
-an authorization code after the first exchange attempt. All state is in memory
-and is discarded when the process exits.
-
-## Automated Test
+Run its tests with:
```powershell
npm run test:opds-pkce-test-server
```
-
-The test covers metadata and issuer discovery, denial, failed PKCE verification,
-code replay, successful token exchange, bearer access to the OPDS feed and
-acquisition, and refresh-token use.
diff --git a/projects/opds-pkce-test-server/server.mjs b/projects/opds-pkce-test-server/server.mjs
index 8bd726baf8..7994ee1ad7 100644
--- a/projects/opds-pkce-test-server/server.mjs
+++ b/projects/opds-pkce-test-server/server.mjs
@@ -5,42 +5,34 @@
// that can be found in the LICENSE file exposed on Github (readium) in the project repository.
// ==LICENSE-END==
-import {
- createHash,
- randomBytes,
- timingSafeEqual,
-} from "node:crypto";
+import { createHash, randomBytes, timingSafeEqual } from "node:crypto";
import { createServer } from "node:http";
import { resolve } from "node:path";
import { fileURLToPath } from "node:url";
export const AUTHENTICATION_TYPE = "http://opds-spec.org/auth/oauth/authorization-code-pkce";
+export const CLIENT_ID = "http://opds-spec.org/auth/client";
+export const REDIRECT_URI = "opds://authorize/";
-const DEFAULT_CLIENT_ID = "http://opds-spec.org/auth/client";
-const DEFAULT_HOST = "127.0.0.1";
+const HOST = "127.0.0.1";
const DEFAULT_PORT = 49152;
-const DEFAULT_REDIRECT_URI = "opds://authorize/";
const AUTHORIZATION_CODE_TTL_MS = 2 * 60 * 1000;
-const ACCESS_TOKEN_TTL_SECONDS = 10 * 60;
+const ACCESS_TOKEN_TTL_MS = 10 * 60 * 1000;
const REFRESH_TOKEN_TTL_MS = 60 * 60 * 1000;
const MAX_FORM_BODY_BYTES = 16 * 1024;
+const PKCE_VERIFIER_REGEXP = /^[A-Za-z0-9\-._~]{43,128}$/;
-function base64Url(buffer) {
- return buffer.toString("base64url");
+function randomToken() {
+ return randomBytes(32).toString("base64url");
}
export function createCodeChallenge(codeVerifier) {
- return base64Url(createHash("sha256").update(codeVerifier, "ascii").digest());
-}
-
-function randomToken(byteLength = 32) {
- return base64Url(randomBytes(byteLength));
+ return createHash("sha256").update(codeVerifier, "ascii").digest("base64url");
}
function constantTimeEqual(left, right) {
const leftBuffer = Buffer.from(left, "ascii");
const rightBuffer = Buffer.from(right, "ascii");
-
return leftBuffer.length === rightBuffer.length && timingSafeEqual(leftBuffer, rightBuffer);
}
@@ -59,16 +51,25 @@ function sendJson(response, statusCode, value, headers = {}) {
"Cache-Control": "no-store",
"Content-Length": Buffer.byteLength(body),
"Content-Type": "application/json; charset=utf-8",
+ "X-Content-Type-Options": "nosniff",
...headers,
});
response.end(body);
}
-function sendHtml(response, statusCode, body) {
- response.writeHead(statusCode, {
+function sendOAuthError(response, statusCode, error, errorDescription) {
+ sendJson(response, statusCode, {
+ error,
+ error_description: errorDescription,
+ });
+}
+
+function sendHtml(response, body) {
+ response.writeHead(200, {
"Cache-Control": "no-store",
"Content-Length": Buffer.byteLength(body),
- "Content-Security-Policy": "default-src 'none'; form-action 'self'; style-src 'unsafe-inline'; base-uri 'none'; frame-ancestors 'none'",
+ "Content-Security-Policy":
+ "default-src 'none'; form-action 'self'; base-uri 'none'; frame-ancestors 'none'",
"Content-Type": "text/html; charset=utf-8",
"Referrer-Policy": "no-referrer",
"X-Content-Type-Options": "nosniff",
@@ -77,17 +78,9 @@ function sendHtml(response, statusCode, body) {
response.end(body);
}
-function sendOAuthError(response, statusCode, error, errorDescription) {
- sendJson(response, statusCode, {
- error,
- error_description: errorDescription,
- });
-}
-
async function readForm(request) {
let body = "";
let byteLength = 0;
-
for await (const chunk of request) {
byteLength += chunk.length;
if (byteLength > MAX_FORM_BODY_BYTES) {
@@ -95,40 +88,38 @@ async function readForm(request) {
}
body += chunk.toString("utf8");
}
-
return new URLSearchParams(body);
}
-function redirectUriWithParams(redirectUri, params) {
- const url = new URL(redirectUri);
+function redirectUriWithParams(params) {
+ const url = new URL(REDIRECT_URI);
for (const [key, value] of Object.entries(params)) {
- if (value !== undefined) {
+ if (value !== undefined && value !== null) {
url.searchParams.set(key, value);
}
}
return url.toString();
}
-function authorizeRequestError(params, config) {
+function authorizationRequestError(params) {
if (params.get("response_type") !== "code") {
return "response_type must be code";
}
- if (params.get("client_id") !== config.clientId) {
+ if (params.get("client_id") !== CLIENT_ID) {
return "client_id is missing or unsupported";
}
- if (params.get("redirect_uri") !== config.redirectUri) {
+ if (params.get("redirect_uri") !== REDIRECT_URI) {
return "redirect_uri is missing or unsupported";
}
if (!params.get("state")) {
- return "state is required by this test server";
+ return "state is required";
}
if (params.get("code_challenge_method") !== "S256") {
return "code_challenge_method must be S256";
}
if (!/^[A-Za-z0-9_-]{43}$/.test(params.get("code_challenge") || "")) {
- return "code_challenge must be a 43-character base64url SHA-256 value";
+ return "code_challenge must be a SHA-256 base64url value";
}
-
return undefined;
}
@@ -137,86 +128,37 @@ function authorizationPage(params) {
"response_type",
"client_id",
"redirect_uri",
- "scope",
"state",
"code_challenge",
"code_challenge_method",
- ].map((name) => ``).join("\n");
+ ]
+ .map((name) => ``)
+ .join("\n");
return `
- Thorium OPDS PKCE test authorization
-
+ OPDS PKCE test authorization
-
Authorize the PKCE test client?
- This local development server does not ask for real credentials.
-
- - Client
${escapeHtml(params.get("client_id"))}
- - Scope
${escapeHtml(params.get("scope") || "opds")}
- - Redirect
${escapeHtml(params.get("redirect_uri"))}
-
+ This local test server does not ask for credentials.
-
-
-`;
-}
-
-function indexPage(origin, config) {
- return `
-
-
-
-
- OPDS PKCE test server
-
-
-
- OPDS Authorization Code + PKCE test server
- Add this protected catalog to the client under test:
- ${escapeHtml(`${origin}/opds/v2/catalog`)}
-
- Expected client ID: ${escapeHtml(config.clientId)}
- Expected redirect URI: ${escapeHtml(config.redirectUri)}
`;
}
function getBearerToken(request) {
- const authorization = request.headers.authorization || "";
- const match = /^Bearer\s+(.+)$/i.exec(authorization);
- return match?.[1];
+ return /^Bearer\s+(.+)$/i.exec(request.headers.authorization || "")?.[1];
}
-export function createPkceTestServer(options = {}) {
- const config = {
- clientId: options.clientId || DEFAULT_CLIENT_ID,
- host: options.host || DEFAULT_HOST,
- port: options.port ?? DEFAULT_PORT,
- redirectUri: options.redirectUri || DEFAULT_REDIRECT_URI,
- };
+export function createPkceTestServer({ port = DEFAULT_PORT } = {}) {
const authorizationCodes = new Map();
const accessTokens = new Map();
const refreshTokens = new Map();
@@ -227,114 +169,51 @@ export function createPkceTestServer(options = {}) {
if (!address || typeof address === "string") {
throw new Error("PKCE test server is not listening");
}
- return `http://${config.host}:${address.port}`;
+ return `http://${HOST}:${address.port}`;
};
+ const authenticationDocument = () => ({
+ id: `${getOrigin()}/auth`,
+ title: "Thorium PKCE Test Catalog",
+ authentication: [
+ {
+ type: AUTHENTICATION_TYPE,
+ links: [
+ { rel: "authenticate", href: `${getOrigin()}/authorize` },
+ { rel: "refresh", href: `${getOrigin()}/token` },
+ ],
+ },
+ ],
+ });
+
const pruneExpiredValues = () => {
const now = Date.now();
- for (const [code, record] of authorizationCodes) {
- if (record.expiresAt <= now) {
- authorizationCodes.delete(code);
- }
- }
- for (const [token, record] of accessTokens) {
- if (record.expiresAt <= now) {
- accessTokens.delete(token);
- }
- }
- for (const [token, record] of refreshTokens) {
- if (record.expiresAt <= now) {
- refreshTokens.delete(token);
+ for (const values of [authorizationCodes, accessTokens, refreshTokens]) {
+ for (const [key, value] of values) {
+ const expiresAt = typeof value === "number" ? value : value.expiresAt;
+ if (expiresAt <= now) {
+ values.delete(key);
+ }
}
}
};
- const authenticationDocument = () => {
- const origin = getOrigin();
- return {
- id: `${origin}/auth`,
- title: "Thorium PKCE Test Catalog",
- description: "Local test service for the OAuth 2.0 Authorization Code flow with PKCE.",
- authentication: [
- {
- type: AUTHENTICATION_TYPE,
- links: [
- {
- rel: "authenticate",
- href: `${origin}/authorize`,
- type: "text/html",
- },
- {
- rel: "token",
- href: `${origin}/token`,
- type: "application/json",
- },
- {
- rel: "refresh",
- href: `${origin}/token`,
- type: "application/json",
- },
- ],
- authorization_server: `${origin}/.well-known/oauth-authorization-server`,
- issuer: origin,
- client_id: config.clientId,
- redirect_uri: config.redirectUri,
- scope: "opds",
- code_challenge_methods_supported: ["S256"],
- },
- ],
- links: [
- {
- rel: "help",
- href: `${origin}/`,
- type: "text/html",
- },
- ],
- };
- };
-
const unauthorized = (response) => {
- const origin = getOrigin();
sendJson(response, 401, authenticationDocument(), {
"Content-Type": "application/opds-authentication+json; charset=utf-8",
- Link: `<${origin}/auth>; rel="http://opds-spec.org/auth/document"; type="application/opds-authentication+json"`,
- "WWW-Authenticate": "Bearer realm=\"Thorium PKCE Test Catalog\"",
+ Link: `<${getOrigin()}/auth>; rel="http://opds-spec.org/auth/document"; type="application/opds-authentication+json"`,
+ "WWW-Authenticate": 'Bearer realm="Thorium PKCE Test Catalog"',
});
};
- const authorizedTokenRecord = (request) => {
- const token = getBearerToken(request);
- if (!token) {
- return undefined;
- }
- const record = accessTokens.get(token);
- if (!record || record.expiresAt <= Date.now()) {
- accessTokens.delete(token);
- return undefined;
- }
- return record;
- };
-
- const issueTokens = (record) => {
+ const issueTokens = () => {
const accessToken = randomToken();
const refreshToken = randomToken();
- const now = Date.now();
- const tokenRecord = {
- clientId: record.clientId,
- expiresAt: now + ACCESS_TOKEN_TTL_SECONDS * 1000,
- scope: record.scope,
- subject: "pkce-test-user",
- };
- accessTokens.set(accessToken, tokenRecord);
- refreshTokens.set(refreshToken, {
- ...tokenRecord,
- expiresAt: now + REFRESH_TOKEN_TTL_MS,
- });
+ accessTokens.set(accessToken, Date.now() + ACCESS_TOKEN_TTL_MS);
+ refreshTokens.set(refreshToken, Date.now() + REFRESH_TOKEN_TTL_MS);
return {
access_token: accessToken,
- expires_in: ACCESS_TOKEN_TTL_SECONDS,
refresh_token: refreshToken,
- scope: record.scope,
token_type: "Bearer",
};
};
@@ -345,21 +224,6 @@ export function createPkceTestServer(options = {}) {
const url = new URL(request.url || "/", origin);
try {
- if (request.method === "GET" && url.pathname === "/") {
- sendHtml(response, 200, indexPage(origin, config));
- return;
- }
-
- if (request.method === "GET" && url.pathname === "/health") {
- sendJson(response, 200, {
- status: "ok",
- active_authorization_codes: authorizationCodes.size,
- active_access_tokens: accessTokens.size,
- active_refresh_tokens: refreshTokens.size,
- });
- return;
- }
-
if (request.method === "GET" && url.pathname === "/auth") {
sendJson(response, 200, authenticationDocument(), {
"Content-Type": "application/opds-authentication+json; charset=utf-8",
@@ -367,46 +231,30 @@ export function createPkceTestServer(options = {}) {
return;
}
- if (request.method === "GET" && url.pathname === "/.well-known/oauth-authorization-server") {
- sendJson(response, 200, {
- issuer: origin,
- authorization_endpoint: `${origin}/authorize`,
- token_endpoint: `${origin}/token`,
- response_types_supported: ["code"],
- grant_types_supported: ["authorization_code", "refresh_token"],
- token_endpoint_auth_methods_supported: ["none"],
- code_challenge_methods_supported: ["S256"],
- authorization_response_iss_parameter_supported: true,
- scopes_supported: ["opds"],
- });
- return;
- }
-
if (request.method === "GET" && url.pathname === "/authorize") {
- const validationError = authorizeRequestError(url.searchParams, config);
- if (validationError) {
- sendOAuthError(response, 400, "invalid_request", validationError);
+ const error = authorizationRequestError(url.searchParams);
+ if (error) {
+ sendOAuthError(response, 400, "invalid_request", error);
return;
}
- sendHtml(response, 200, authorizationPage(url.searchParams));
+ sendHtml(response, authorizationPage(url.searchParams));
return;
}
if (request.method === "POST" && url.pathname === "/authorize") {
const params = await readForm(request);
- const validationError = authorizeRequestError(params, config);
- if (validationError) {
- sendOAuthError(response, 400, "invalid_request", validationError);
+ const error = authorizationRequestError(params);
+ if (error) {
+ sendOAuthError(response, 400, "invalid_request", error);
return;
}
if (params.get("decision") !== "approve") {
response.writeHead(303, {
"Cache-Control": "no-store",
- Location: redirectUriWithParams(config.redirectUri, {
+ Location: redirectUriWithParams({
error: "access_denied",
error_description: "The test user denied the authorization request.",
- iss: origin,
state: params.get("state"),
}),
});
@@ -417,19 +265,11 @@ export function createPkceTestServer(options = {}) {
const code = randomToken();
authorizationCodes.set(code, {
challenge: params.get("code_challenge"),
- clientId: config.clientId,
expiresAt: Date.now() + AUTHORIZATION_CODE_TTL_MS,
- redirectUri: config.redirectUri,
- scope: params.get("scope") || "opds",
});
response.writeHead(303, {
"Cache-Control": "no-store",
- Location: redirectUriWithParams(config.redirectUri, {
- code,
- id: `${origin}/auth`,
- iss: origin,
- state: params.get("state"),
- }),
+ Location: redirectUriWithParams({ code, state: params.get("state") }),
});
response.end();
return;
@@ -438,7 +278,7 @@ export function createPkceTestServer(options = {}) {
if (request.method === "POST" && url.pathname === "/token") {
const params = await readForm(request);
if (params.get("client_secret")) {
- sendOAuthError(response, 401, "invalid_client", "This public-client test server does not accept a client_secret.");
+ sendOAuthError(response, 401, "invalid_client", "The shared OPDS client does not use a secret.");
return;
}
@@ -446,48 +286,38 @@ export function createPkceTestServer(options = {}) {
const code = params.get("code") || "";
const record = authorizationCodes.get(code);
authorizationCodes.delete(code);
-
if (!record || record.expiresAt <= Date.now()) {
- sendOAuthError(response, 400, "invalid_grant", "The authorization code is invalid, expired, or already used.");
+ sendOAuthError(response, 400, "invalid_grant", "The authorization code is invalid or expired.");
return;
}
- if (params.get("client_id") !== record.clientId || params.get("redirect_uri") !== record.redirectUri) {
- sendOAuthError(response, 400, "invalid_grant", "The client_id or redirect_uri does not match the authorization request.");
+ if (params.get("client_id") !== CLIENT_ID || params.get("redirect_uri") !== REDIRECT_URI) {
+ sendOAuthError(response, 400, "invalid_grant", "The client_id or redirect_uri does not match.");
return;
}
-
const verifier = params.get("code_verifier") || "";
- if (!/^[A-Za-z0-9\-._~]{43,128}$/.test(verifier)) {
- sendOAuthError(response, 400, "invalid_grant", "The code_verifier is malformed.");
- return;
- }
- if (!constantTimeEqual(createCodeChallenge(verifier), record.challenge)) {
+ if (
+ !PKCE_VERIFIER_REGEXP.test(verifier) ||
+ !constantTimeEqual(createCodeChallenge(verifier), record.challenge)
+ ) {
sendOAuthError(response, 400, "invalid_grant", "PKCE verification failed.");
return;
}
-
- sendJson(response, 200, issueTokens(record));
+ sendJson(response, 200, issueTokens());
return;
}
if (params.get("grant_type") === "refresh_token") {
const refreshToken = params.get("refresh_token") || "";
- const record = refreshTokens.get(refreshToken);
- if (!record || record.expiresAt <= Date.now() || params.get("client_id") !== record.clientId) {
+ const expiresAt = refreshTokens.get(refreshToken);
+ if (!expiresAt || expiresAt <= Date.now() || params.get("client_id") !== CLIENT_ID) {
refreshTokens.delete(refreshToken);
sendOAuthError(response, 400, "invalid_grant", "The refresh token is invalid or expired.");
return;
}
-
const accessToken = randomToken();
- accessTokens.set(accessToken, {
- ...record,
- expiresAt: Date.now() + ACCESS_TOKEN_TTL_SECONDS * 1000,
- });
+ accessTokens.set(accessToken, Date.now() + ACCESS_TOKEN_TTL_MS);
sendJson(response, 200, {
access_token: accessToken,
- expires_in: ACCESS_TOKEN_TTL_SECONDS,
- scope: record.scope,
token_type: "Bearer",
});
return;
@@ -497,155 +327,73 @@ export function createPkceTestServer(options = {}) {
return;
}
- if (request.method === "GET" && url.pathname === "/callback") {
- const outcome = url.searchParams.has("error") ? "Authorization denied" : "Authorization callback received";
- sendHtml(response, 200, `${outcome}${outcome}
You can close this browser tab.
`);
- return;
- }
-
if (request.method === "GET" && url.pathname === "/opds/v2/catalog") {
- if (!authorizedTokenRecord(request)) {
+ const token = getBearerToken(request);
+ const expiresAt = token ? accessTokens.get(token) : undefined;
+ if (!expiresAt || expiresAt <= Date.now()) {
+ if (token) {
+ accessTokens.delete(token);
+ }
unauthorized(response);
return;
}
-
- sendJson(response, 200, {
- metadata: {
- title: "Thorium PKCE Test Catalog",
- modified: new Date().toISOString(),
- numberOfItems: 1,
+ sendJson(
+ response,
+ 200,
+ {
+ metadata: { title: "Thorium PKCE Test Catalog" },
+ links: [{ rel: "self", href: `${origin}/opds/v2/catalog`, type: "application/opds+json" }],
},
- links: [
- {
- rel: "self",
- href: `${origin}/opds/v2/catalog`,
- type: "application/opds+json",
- },
- {
- rel: "start",
- href: `${origin}/opds/v2/catalog`,
- type: "application/opds+json",
- },
- ],
- publications: [
- {
- metadata: {
- identifier: "urn:thorium:pkce-test-publication",
- title: "PKCE authentication succeeded",
- modified: "2026-01-01T00:00:00Z",
- language: "en",
- },
- links: [
- {
- rel: "http://opds-spec.org/acquisition/open-access",
- href: `${origin}/publication.txt`,
- type: "text/plain",
- },
- ],
- },
- ],
- }, {
- "Content-Type": "application/opds+json; charset=utf-8",
- });
- return;
- }
-
- if (request.method === "GET" && url.pathname === "/publication.txt") {
- if (!authorizedTokenRecord(request)) {
- unauthorized(response);
- return;
- }
- const body = "Thorium successfully used an access token obtained with Authorization Code + PKCE.\n";
- response.writeHead(200, {
- "Cache-Control": "no-store",
- "Content-Disposition": "attachment; filename=pkce-authentication-succeeded.txt",
- "Content-Length": Buffer.byteLength(body),
- "Content-Type": "text/plain; charset=utf-8",
- });
- response.end(body);
+ {
+ "Content-Type": "application/opds+json; charset=utf-8",
+ },
+ );
return;
}
- sendJson(response, 404, {
- error: "not_found",
- });
+ sendJson(response, 404, { error: "not_found" });
} catch (error) {
- sendJson(response, error.message === "Form body is too large" ? 413 : 500, {
- error: "server_error",
- error_description: error.message,
- });
- }
- });
-
- const listen = () => new Promise((resolveListen, rejectListen) => {
- const onError = (error) => rejectListen(error);
- server.once("error", onError);
- server.listen(config.port, config.host, () => {
- server.off("error", onError);
- resolveListen();
- });
- });
-
- const close = () => new Promise((resolveClose, rejectClose) => {
- server.close((error) => {
- if (error) {
- rejectClose(error);
- return;
+ if (!response.headersSent) {
+ sendOAuthError(
+ response,
+ 400,
+ "invalid_request",
+ error instanceof Error ? error.message : String(error),
+ );
+ } else {
+ response.destroy(error instanceof Error ? error : new Error(String(error)));
}
- resolveClose();
- });
+ }
});
return {
- close,
- config,
- get origin() {
- return getOrigin();
- },
- listen,
- server,
+ listen: () =>
+ new Promise((resolveListen, rejectListen) => {
+ server.once("error", rejectListen);
+ server.listen(port, HOST, () => {
+ server.off("error", rejectListen);
+ resolveListen({
+ close: () =>
+ new Promise((resolveClose, rejectClose) => {
+ server.close((error) => (error ? rejectClose(error) : resolveClose()));
+ }),
+ origin: getOrigin(),
+ });
+ });
+ }),
};
}
-export async function startPkceTestServer(options = {}) {
- const app = createPkceTestServer(options);
- await app.listen();
- return app;
+export async function startPkceTestServer(options) {
+ return createPkceTestServer(options).listen();
}
-function installShutdownHandlers(app) {
- let closing = false;
- const shutdown = async (signal) => {
- if (closing) {
- return;
- }
- closing = true;
- console.log(`OPDS PKCE test server shutting down: ${signal}`);
- try {
- await app.close();
- process.exit(0);
- } catch (error) {
- console.error("OPDS PKCE test server shutdown failed", error);
- process.exit(1);
- }
- };
- process.once("SIGINT", () => void shutdown("SIGINT"));
- process.once("SIGTERM", () => void shutdown("SIGTERM"));
-}
-
-const isDirectRun = process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url);
-if (isDirectRun) {
- const cliPort = Number(process.argv[2]);
- const app = await startPkceTestServer({
- clientId: process.env.OPDS_PKCE_CLIENT_ID || DEFAULT_CLIENT_ID,
- host: DEFAULT_HOST,
- port: Number.isInteger(cliPort) && cliPort >= 0
- ? cliPort
- : Number(process.env.OPDS_PKCE_PORT) || DEFAULT_PORT,
- redirectUri: process.env.OPDS_PKCE_REDIRECT_URI || DEFAULT_REDIRECT_URI,
- });
- console.log(`OPDS PKCE test server: ${app.origin}/`);
- console.log(`Protected OPDS 2 catalog: ${app.origin}/opds/v2/catalog`);
- console.log(`Redirect URI: ${app.config.redirectUri}`);
- installShutdownHandlers(app);
+const directRun = process.argv[1] && resolve(process.argv[1]) === resolve(fileURLToPath(import.meta.url));
+if (directRun) {
+ const port = process.argv[2] === undefined ? DEFAULT_PORT : Number.parseInt(process.argv[2], 10);
+ if (!Number.isInteger(port) || port < 0 || port > 65535) {
+ throw new Error("The port must be an integer between 0 and 65535.");
+ }
+ const app = await startPkceTestServer({ port });
+ process.stdout.write(`OPDS PKCE test server: ${app.origin}/opds/v2/catalog\n`);
}
diff --git a/projects/opds-pkce-test-server/server.test.mjs b/projects/opds-pkce-test-server/server.test.mjs
index 12a2d07639..49a7858c65 100644
--- a/projects/opds-pkce-test-server/server.test.mjs
+++ b/projects/opds-pkce-test-server/server.test.mjs
@@ -9,22 +9,12 @@ import assert from "node:assert/strict";
import { randomBytes } from "node:crypto";
import { after, before, test } from "node:test";
-import {
- AUTHENTICATION_TYPE,
- createCodeChallenge,
- startPkceTestServer,
-} from "./server.mjs";
-
-const clientId = "http://opds-spec.org/auth/client";
-const redirectUri = "opds://authorize/";
+import { AUTHENTICATION_TYPE, CLIENT_ID, REDIRECT_URI, createCodeChallenge, startPkceTestServer } from "./server.mjs";
+
let app;
before(async () => {
- app = await startPkceTestServer({
- clientId,
- port: 0,
- redirectUri,
- });
+ app = await startPkceTestServer({ port: 0 });
});
after(async () => {
@@ -35,23 +25,25 @@ function newPkceTransaction() {
const verifier = randomBytes(32).toString("base64url");
return {
challenge: createCodeChallenge(verifier),
- state: randomBytes(16).toString("base64url"),
+ state: randomBytes(32).toString("base64url"),
verifier,
};
}
-async function authorize(transaction, decision = "approve") {
- const params = new URLSearchParams({
+function authorizationParams(transaction) {
+ return new URLSearchParams({
response_type: "code",
- client_id: clientId,
- redirect_uri: redirectUri,
- scope: "opds",
- state: transaction.state,
+ client_id: CLIENT_ID,
+ redirect_uri: REDIRECT_URI,
code_challenge: transaction.challenge,
code_challenge_method: "S256",
+ state: transaction.state,
});
- const authorizeUrl = `${app.origin}/authorize?${params}`;
- const pageResponse = await fetch(authorizeUrl);
+}
+
+async function authorize(transaction, decision = "approve") {
+ const params = authorizationParams(transaction);
+ const pageResponse = await fetch(`${app.origin}/authorize?${params}`);
assert.equal(pageResponse.status, 200);
assert.match(await pageResponse.text(), /Authorize the PKCE test client/);
@@ -62,128 +54,109 @@ async function authorize(transaction, decision = "approve") {
redirect: "manual",
});
assert.equal(response.status, 303);
- const location = response.headers.get("location");
- assert.ok(location);
- return new URL(location);
+ return new URL(response.headers.get("location"));
}
-async function exchangeCode(code, verifier) {
+function exchangeCode(code, verifier, extra = {}) {
return fetch(`${app.origin}/token`, {
body: new URLSearchParams({
grant_type: "authorization_code",
- client_id: clientId,
- redirect_uri: redirectUri,
code,
+ redirect_uri: REDIRECT_URI,
+ client_id: CLIENT_ID,
code_verifier: verifier,
+ ...extra,
}),
method: "POST",
});
}
-test("exposes the OPDS authentication document and OAuth metadata", async () => {
- const protectedResponse = await fetch(`${app.origin}/opds/v2/catalog`);
- assert.equal(protectedResponse.status, 401);
- assert.match(protectedResponse.headers.get("content-type"), /^application\/opds-authentication\+json/);
- assert.match(protectedResponse.headers.get("link"), /opds-spec\.org\/auth\/document/);
+test("advertises only the proposed OPDS PKCE fields", async () => {
+ const response = await fetch(`${app.origin}/opds/v2/catalog`);
+ assert.equal(response.status, 401);
+ assert.match(response.headers.get("content-type"), /^application\/opds-authentication\+json/);
- const document = await protectedResponse.json();
+ const document = await response.json();
+ assert.deepEqual(Object.keys(document.authentication[0]).sort(), ["links", "type"]);
assert.equal(document.authentication[0].type, AUTHENTICATION_TYPE);
- assert.equal(document.authentication[0].client_id, clientId);
- assert.equal(document.authentication[0].redirect_uri, redirectUri);
- assert.deepEqual(document.authentication[0].code_challenge_methods_supported, ["S256"]);
- assert.equal(
- document.authentication[0].links.find((link) => link.rel === "token")?.href,
- `${app.origin}/token`,
- );
-
- const metadataResponse = await fetch(`${app.origin}/.well-known/oauth-authorization-server`);
- assert.equal(metadataResponse.status, 200);
- const metadata = await metadataResponse.json();
- assert.equal(metadata.issuer, app.origin);
- assert.equal(metadata.authorization_endpoint, `${app.origin}/authorize`);
- assert.equal(metadata.token_endpoint, `${app.origin}/token`);
- assert.equal(metadata.authorization_response_iss_parameter_supported, true);
- assert.deepEqual(metadata.code_challenge_methods_supported, ["S256"]);
+ assert.deepEqual(document.authentication[0].links, [
+ { rel: "authenticate", href: `${app.origin}/authorize` },
+ { rel: "refresh", href: `${app.origin}/token` },
+ ]);
+
+ for (const removedPath of ["/.well-known/oauth-authorization-server", "/health", "/publication.txt"]) {
+ assert.equal((await fetch(`${app.origin}${removedPath}`)).status, 404);
+ }
});
-test("rejects unsupported or malformed authorization requests", async () => {
- const response = await fetch(`${app.origin}/authorize?response_type=token`);
- assert.equal(response.status, 400);
- assert.equal((await response.json()).error, "invalid_request");
+test("requires the fixed client, redirect URI, and S256", async () => {
+ const transaction = newPkceTransaction();
+ const params = authorizationParams(transaction);
+
+ params.set("client_id", "other-client");
+ assert.equal((await fetch(`${app.origin}/authorize?${params}`)).status, 400);
+ params.set("client_id", CLIENT_ID);
+
+ params.set("redirect_uri", "https://attacker.example/callback");
+ assert.equal((await fetch(`${app.origin}/authorize?${params}`)).status, 400);
+ params.set("redirect_uri", REDIRECT_URI);
+
+ params.set("code_challenge_method", "plain");
+ assert.equal((await fetch(`${app.origin}/authorize?${params}`)).status, 400);
});
-test("returns an OAuth access_denied callback", async () => {
+test("returns denial with the original state", async () => {
const transaction = newPkceTransaction();
const callback = await authorize(transaction, "deny");
- assert.equal(callback.origin, "null");
assert.equal(callback.protocol, "opds:");
assert.equal(callback.searchParams.get("error"), "access_denied");
- assert.equal(callback.searchParams.get("iss"), app.origin);
assert.equal(callback.searchParams.get("state"), transaction.state);
+ assert.equal(callback.searchParams.has("iss"), false);
});
-test("invalidates a code after a failed PKCE verification", async () => {
+test("invalidates a code after failed PKCE verification", async () => {
const transaction = newPkceTransaction();
- const callback = await authorize(transaction);
- const code = callback.searchParams.get("code");
+ const code = (await authorize(transaction)).searchParams.get("code");
assert.ok(code);
const wrongVerifier = randomBytes(32).toString("base64url");
- const failedResponse = await exchangeCode(code, wrongVerifier);
- assert.equal(failedResponse.status, 400);
- assert.equal((await failedResponse.json()).error, "invalid_grant");
-
- const retryResponse = await exchangeCode(code, transaction.verifier);
- assert.equal(retryResponse.status, 400);
- assert.equal((await retryResponse.json()).error, "invalid_grant");
+ assert.equal((await exchangeCode(code, wrongVerifier)).status, 400);
+ assert.equal((await exchangeCode(code, transaction.verifier)).status, 400);
});
-test("exchanges a valid code once and accepts the resulting bearer token", async () => {
+test("exchanges a code, protects the catalog, and refreshes at the same endpoint", async () => {
const transaction = newPkceTransaction();
const callback = await authorize(transaction);
const code = callback.searchParams.get("code");
assert.ok(code);
assert.equal(callback.searchParams.get("state"), transaction.state);
- assert.equal(callback.searchParams.get("id"), `${app.origin}/auth`);
- assert.equal(callback.searchParams.get("iss"), app.origin);
+ assert.deepEqual([...callback.searchParams.keys()].sort(), ["code", "state"]);
+
+ const secretResponse = await exchangeCode(code, transaction.verifier, { client_secret: "secret" });
+ assert.equal(secretResponse.status, 401);
- const tokenResponse = await exchangeCode(code, transaction.verifier);
+ const retryTransaction = newPkceTransaction();
+ const retryCode = (await authorize(retryTransaction)).searchParams.get("code");
+ assert.ok(retryCode);
+ const tokenResponse = await exchangeCode(retryCode, retryTransaction.verifier);
assert.equal(tokenResponse.status, 200);
assert.match(tokenResponse.headers.get("cache-control"), /no-store/);
- const tokenDocument = await tokenResponse.json();
- assert.equal(tokenDocument.token_type, "Bearer");
- assert.equal(tokenDocument.scope, "opds");
- assert.ok(tokenDocument.access_token);
- assert.ok(tokenDocument.refresh_token);
-
- const reuseResponse = await exchangeCode(code, transaction.verifier);
- assert.equal(reuseResponse.status, 400);
- assert.equal((await reuseResponse.json()).error, "invalid_grant");
+ const token = await tokenResponse.json();
+ assert.ok(token.access_token);
+ assert.ok(token.refresh_token);
+ assert.equal(token.token_type, "Bearer");
const catalogResponse = await fetch(`${app.origin}/opds/v2/catalog`, {
- headers: {
- Authorization: `Bearer ${tokenDocument.access_token}`,
- },
+ headers: { Authorization: `Bearer ${token.access_token}` },
});
assert.equal(catalogResponse.status, 200);
assert.match(catalogResponse.headers.get("content-type"), /^application\/opds\+json/);
- const catalog = await catalogResponse.json();
- assert.equal(catalog.metadata.title, "Thorium PKCE Test Catalog");
- assert.equal(catalog.publications[0].metadata.title, "PKCE authentication succeeded");
-
- const publicationResponse = await fetch(`${app.origin}/publication.txt`, {
- headers: {
- Authorization: `Bearer ${tokenDocument.access_token}`,
- },
- });
- assert.equal(publicationResponse.status, 200);
- assert.match(await publicationResponse.text(), /Authorization Code \+ PKCE/);
const refreshResponse = await fetch(`${app.origin}/token`, {
body: new URLSearchParams({
grant_type: "refresh_token",
- client_id: clientId,
- refresh_token: tokenDocument.refresh_token,
+ client_id: CLIENT_ID,
+ refresh_token: token.refresh_token,
}),
method: "POST",
});
diff --git a/src/main/network/http.ts b/src/main/network/http.ts
index 6b7d62c9ce..148962ac53 100644
--- a/src/main/network/http.ts
+++ b/src/main/network/http.ts
@@ -36,6 +36,7 @@ import {
IHttpGetResult, THttpGetCallback, THttpOptions, THttpResponse,
} from "readium-desktop/common/utils/http";
import { decryptPersist, encryptPersist } from "readium-desktop/main/fs/persistCrypto";
+import { createOpdsPkceRefreshTokenRequest } from "readium-desktop/main/network/opdsPkce";
import { tryCatch, tryCatchSync } from "readium-desktop/utils/tryCatch";
import { diMainGet, opdsAuthFilePath } from "../di";
@@ -686,11 +687,7 @@ const httpGetUnauthorizedRefresh =
// refresh request. Keep the JSON body for legacy OPDS credentials that predate clientId.
if (auth.clientId) {
(options.headers as Headers).set("Content-Type", "application/x-www-form-urlencoded");
- options.body = new URLSearchParams({
- client_id: auth.clientId,
- grant_type: "refresh_token",
- refresh_token: refreshToken,
- }).toString();
+ options.body = createOpdsPkceRefreshTokenRequest(refreshToken, auth.clientId);
} else {
(options.headers as Headers).set("Content-Type", "application/json");
options.body = JSON.stringify({
diff --git a/src/main/network/opdsPkce.ts b/src/main/network/opdsPkce.ts
index 01d7d8e724..5d9b8d734e 100644
--- a/src/main/network/opdsPkce.ts
+++ b/src/main/network/opdsPkce.ts
@@ -9,49 +9,35 @@ import { createHash, randomBytes } from "node:crypto";
export const OPDS_AUTHORIZATION_CODE_PKCE_TYPE =
"http://opds-spec.org/auth/oauth/authorization-code-pkce";
+export const OPDS_OAUTH_CLIENT_ID = "http://opds-spec.org/auth/client";
+export const OPDS_OAUTH_REDIRECT_URI = "opds://authorize/";
const PKCE_TRANSACTION_MAX_AGE_MS = 5 * 60 * 1000;
const PKCE_VERIFIER_REGEXP = /^[A-Za-z0-9\-._~]{43,128}$/;
-function assertSecureOAuthEndpoint(value: string, name: string): URL {
+function assertSecureOAuthEndpoint(value: string, name: string, allowInsecureLoopback: boolean): URL {
const url = new URL(value);
const isLoopbackHttp = url.protocol === "http:" &&
(url.hostname === "localhost" || url.hostname === "[::1]" || /^127(?:\.\d{1,3}){3}$/.test(url.hostname));
- if (url.protocol !== "https:" && !isLoopbackHttp) {
- throw new Error(`The PKCE ${name} must use HTTPS, except on a loopback address.`);
+ if (url.protocol !== "https:" && !(allowInsecureLoopback && isLoopbackHttp)) {
+ const exception = allowInsecureLoopback ? ", except on a loopback address" : "";
+ throw new Error(`The PKCE ${name} must use HTTPS${exception}.`);
}
return url;
}
export interface IOpdsPkceConfiguration {
- authenticationDocumentId?: string;
+ allowInsecureLoopback?: boolean;
authorizationUrl: string;
- clientId: string;
- redirectUri: string;
- scope?: string;
tokenUrl: string;
- issuer?: string;
- application?: string;
- applicationVersion?: string;
-}
-
-export interface IOpdsPkceAuthorizationServerConfiguration {
- authorizationServerMetadataUrl: string;
- expectedAuthorizationUrl?: string;
- expectedIssuer: string;
- expectedTokenUrl?: string;
-}
-
-export interface IOpdsPkceAuthorizationServerMetadata {
- authorizationEndpoint: string;
- issuer: string;
- tokenEndpoint: string;
}
export interface IOpdsPkceTransaction extends IOpdsPkceConfiguration {
authorizationRequestUrl: string;
+ clientId: typeof OPDS_OAUTH_CLIENT_ID;
codeVerifier: string;
createdAt: number;
+ redirectUri: typeof OPDS_OAUTH_REDIRECT_URI;
state: string;
}
@@ -59,21 +45,16 @@ export interface IOpdsPkceCallback {
code?: string;
error?: string;
error_description?: string;
- id?: string;
- iss?: string;
state?: string;
}
export interface IOpdsPkceTokenResponse {
accessToken: string;
- expiresIn?: number;
refreshToken?: string;
- scope?: string;
tokenType: string;
}
export type TOpdsPkceTokenPost = (url: string, body: string) => Promise;
-export type TOpdsPkceMetadataGet = (url: string) => Promise;
export function getSafeOpdsAuthUrlForLog(value: string): string {
try {
@@ -87,68 +68,6 @@ export function getSafeOpdsAuthUrlForLog(value: string): string {
}
}
-function assertMatchingEndpoint(actual: URL, expected: string | undefined, name: string) {
- if (expected && actual.href !== assertSecureOAuthEndpoint(expected, name).href) {
- throw new Error(`The PKCE ${name} does not match the authorization server metadata.`);
- }
-}
-
-export async function loadOpdsPkceAuthorizationServerMetadata(
- configuration: IOpdsPkceAuthorizationServerConfiguration,
- getMetadata: TOpdsPkceMetadataGet,
-): Promise {
- const metadataUrl = assertSecureOAuthEndpoint(
- configuration.authorizationServerMetadataUrl,
- "authorization server metadata URL",
- );
- const expectedIssuerUrl = assertSecureOAuthEndpoint(configuration.expectedIssuer, "issuer");
- if (expectedIssuerUrl.search || expectedIssuerUrl.hash) {
- throw new Error("The PKCE issuer must not contain a query or fragment.");
- }
- if (metadataUrl.origin !== expectedIssuerUrl.origin) {
- throw new Error("The PKCE authorization server metadata URL must have the same origin as the issuer.");
- }
-
- const value = await getMetadata(metadataUrl.href);
- if (!value || typeof value !== "object") {
- throw new Error("The OAuth authorization server returned invalid metadata.");
- }
-
- const metadata = value as Record;
- if (typeof metadata.issuer !== "string" || metadata.issuer !== configuration.expectedIssuer) {
- throw new Error("The OAuth authorization server metadata issuer does not match.");
- }
- if (metadata.authorization_response_iss_parameter_supported !== true) {
- throw new Error("The OAuth authorization server must support the authorization response iss parameter.");
- }
- if (!Array.isArray(metadata.code_challenge_methods_supported) ||
- !metadata.code_challenge_methods_supported.includes("S256")) {
- throw new Error("The OAuth authorization server does not support PKCE S256.");
- }
- if (typeof metadata.authorization_endpoint !== "string" ||
- typeof metadata.token_endpoint !== "string") {
- throw new Error("The OAuth authorization server metadata is missing its endpoints.");
- }
-
- const authorizationEndpoint = assertSecureOAuthEndpoint(
- metadata.authorization_endpoint,
- "authorization endpoint",
- );
- const tokenEndpoint = assertSecureOAuthEndpoint(metadata.token_endpoint, "token endpoint");
- assertMatchingEndpoint(
- authorizationEndpoint,
- configuration.expectedAuthorizationUrl,
- "authorization endpoint",
- );
- assertMatchingEndpoint(tokenEndpoint, configuration.expectedTokenUrl, "token endpoint");
-
- return {
- authorizationEndpoint: authorizationEndpoint.href,
- issuer: metadata.issuer,
- tokenEndpoint: tokenEndpoint.href,
- };
-}
-
export function createOpdsPkceCodeChallenge(codeVerifier: string): string {
if (!PKCE_VERIFIER_REGEXP.test(codeVerifier)) {
throw new Error("The PKCE code verifier must contain 43 to 128 RFC 7636 unreserved characters.");
@@ -164,39 +83,36 @@ export function createOpdsPkceTransaction(
createdAt = Date.now(),
): IOpdsPkceTransaction {
if (!configuration.authorizationUrl || !configuration.tokenUrl) {
- throw new Error("The PKCE authorization and token endpoints are required.");
- }
- if (!configuration.clientId || !configuration.redirectUri) {
- throw new Error("The PKCE client_id and redirect_uri are required.");
+ throw new Error("The PKCE authenticate and refresh links are required.");
}
- const authorizationUrl = assertSecureOAuthEndpoint(configuration.authorizationUrl, "authorization endpoint");
- assertSecureOAuthEndpoint(configuration.tokenUrl, "token endpoint");
- new URL(configuration.redirectUri);
+ const authorizationUrl = assertSecureOAuthEndpoint(
+ configuration.authorizationUrl,
+ "authorization endpoint",
+ !!configuration.allowInsecureLoopback,
+ );
+ assertSecureOAuthEndpoint(
+ configuration.tokenUrl,
+ "token endpoint",
+ !!configuration.allowInsecureLoopback,
+ );
const codeVerifier = randomBytes(32).toString("base64url");
const state = randomBytes(32).toString("base64url");
authorizationUrl.searchParams.set("response_type", "code");
- authorizationUrl.searchParams.set("client_id", configuration.clientId);
- authorizationUrl.searchParams.set("redirect_uri", configuration.redirectUri);
+ authorizationUrl.searchParams.set("client_id", OPDS_OAUTH_CLIENT_ID);
+ authorizationUrl.searchParams.set("redirect_uri", OPDS_OAUTH_REDIRECT_URI);
authorizationUrl.searchParams.set("code_challenge", createOpdsPkceCodeChallenge(codeVerifier));
authorizationUrl.searchParams.set("code_challenge_method", "S256");
authorizationUrl.searchParams.set("state", state);
- if (configuration.scope) {
- authorizationUrl.searchParams.set("scope", configuration.scope);
- }
- if (configuration.application) {
- authorizationUrl.searchParams.set("application", configuration.application);
- }
- if (configuration.applicationVersion) {
- authorizationUrl.searchParams.set("application_version", configuration.applicationVersion);
- }
return {
...configuration,
authorizationRequestUrl: authorizationUrl.toString(),
+ clientId: OPDS_OAUTH_CLIENT_ID,
codeVerifier,
createdAt,
+ redirectUri: OPDS_OAUTH_REDIRECT_URI,
state,
};
}
@@ -212,22 +128,10 @@ export function validateOpdsPkceCallback(
if (!callback.state || callback.state !== transaction.state) {
throw new Error("The OAuth callback state does not match the PKCE transaction.");
}
- if (transaction.issuer) {
- if (!callback.iss) {
- throw new Error("The OAuth callback does not contain the expected issuer.");
- }
- if (callback.iss !== transaction.issuer) {
- throw new Error("The OAuth callback issuer does not match.");
- }
- }
if (callback.error) {
const description = callback.error_description ? `: ${callback.error_description}` : "";
throw new Error(`OAuth authorization failed (${callback.error})${description}`);
}
- if (callback.id && transaction.authenticationDocumentId &&
- callback.id !== transaction.authenticationDocumentId) {
- throw new Error("The OAuth callback authentication document identifier does not match.");
- }
if (!callback.code) {
throw new Error("The OAuth callback does not contain an authorization code.");
}
@@ -241,13 +145,21 @@ export function createOpdsPkceTokenRequest(
): string {
return new URLSearchParams({
grant_type: "authorization_code",
- client_id: transaction.clientId,
- redirect_uri: transaction.redirectUri,
code: authorizationCode,
+ redirect_uri: transaction.redirectUri,
+ client_id: transaction.clientId,
code_verifier: transaction.codeVerifier,
}).toString();
}
+export function createOpdsPkceRefreshTokenRequest(refreshToken: string, clientId = OPDS_OAUTH_CLIENT_ID): string {
+ return new URLSearchParams({
+ grant_type: "refresh_token",
+ refresh_token: refreshToken,
+ client_id: clientId,
+ }).toString();
+}
+
export function parseOpdsPkceTokenResponse(value: unknown): IOpdsPkceTokenResponse {
if (!value || typeof value !== "object") {
throw new Error("The OAuth token endpoint returned an invalid response.");
@@ -266,9 +178,7 @@ export function parseOpdsPkceTokenResponse(value: unknown): IOpdsPkceTokenRespon
return {
accessToken: response.access_token,
- expiresIn: typeof response.expires_in === "number" ? response.expires_in : undefined,
refreshToken: typeof response.refresh_token === "string" ? response.refresh_token : undefined,
- scope: typeof response.scope === "string" ? response.scope : undefined,
tokenType: typeof response.token_type === "string" && response.token_type
? response.token_type
: "Bearer",
diff --git a/src/main/redux/sagas/auth.ts b/src/main/redux/sagas/auth.ts
index 45f6d0b9e7..9c09b290b3 100644
--- a/src/main/redux/sagas/auth.ts
+++ b/src/main/redux/sagas/auth.ts
@@ -34,7 +34,6 @@ import {
deleteAuthenticationToken,
getAuthenticationToken,
httpGet,
- httpGetWithAuth,
httpPost,
httpSetAuthenticationToken,
IOpdsAuthenticationToken, wipeAuthenticationTokenStorage,
@@ -43,10 +42,10 @@ import { ContentType } from "readium-desktop/utils/contentType";
import {
IOpdsPkceTransaction,
OPDS_AUTHORIZATION_CODE_PKCE_TYPE,
+ OPDS_OAUTH_CLIENT_ID,
createOpdsPkceTransaction,
exchangeOpdsPkceAuthorizationCode,
getSafeOpdsAuthUrlForLog,
- loadOpdsPkceAuthorizationServerMetadata,
} from "readium-desktop/main/network/opdsPkce";
import { tryCatch, tryCatchSync } from "readium-desktop/utils/tryCatch";
// eslint-disable-next-line local-rules/typed-redux-saga-use-typed-effects
@@ -91,11 +90,11 @@ const filename_ = "readium-desktop:main:saga:auth";
const debug = debug_(filename_);
debug("_");
-type TLinkType = "refresh" | "authenticate" | "token";
+type TLinkType = "refresh" | "authenticate";
type TLabelName = "login" | "password";
type TDigestInfo = "realm" | "nonce" | "qop" | "algorithm";
type TAuthName = "id" | "access_token" | "refresh_token" | "token_type"
- | "code" | "state" | "error" | "error_description" | "iss";
+ | "code" | "state" | "error" | "error_description";
type TAuthenticationType = typeof OPDS_AUTHORIZATION_CODE_PKCE_TYPE
| "http://opds-spec.org/auth/oauth/password"
| "http://opds-spec.org/auth/oauth/password/apiapp"
@@ -117,7 +116,6 @@ const AUTHENTICATION_TYPE: TAuthenticationType[] = [
];
const LINK_TYPE: TLinkType[] = [
- "token",
"refresh",
"authenticate",
];
@@ -147,65 +145,16 @@ const opdsAuthFlow =
let pkceTransaction: IOpdsPkceTransaction | undefined;
if (authParsed.authenticationType === OPDS_AUTHORIZATION_CODE_PKCE_TYPE) {
- const registeredRedirectUri = `${URL_PROTOCOL_OPDS}://${URL_HOST_OPDS_AUTH}/`;
- if (authParsed.pkce?.redirectUri !== registeredRedirectUri) {
- debug("OPDS PKCE redirect_uri does not match Thorium's registered callback URI");
- return;
- }
- if (authParsed.pkce?.codeChallengeMethodsSupported &&
- !authParsed.pkce.codeChallengeMethodsSupported.includes("S256")) {
- debug("OPDS PKCE authentication provider does not advertise S256 support");
- return;
- }
- if (!authParsed.pkce?.authorizationServer || !authParsed.pkce.issuer) {
- debug("OPDS PKCE authentication provider does not advertise authorization server metadata");
- return;
- }
- const pkceMetadata = yield* callTyped(() => tryCatch(
- () => loadOpdsPkceAuthorizationServerMetadata(
- {
- authorizationServerMetadataUrl: authParsed.pkce.authorizationServer,
- expectedAuthorizationUrl: authParsed.links?.authenticate?.url,
- expectedIssuer: authParsed.pkce.issuer,
- expectedTokenUrl: authParsed.links?.token?.url,
- },
- async (metadataUrl) => {
- const headers = new Headers();
- headers.set("Accept", "application/json");
- const response = await httpGetWithAuth(false)(metadataUrl, { headers });
- if (!response.isSuccess || !response.response) {
- throw new Error(
- `OAuth authorization server metadata failed with HTTP ${response.statusCode || 0}`,
- );
- }
- if (response.response.url && new URL(response.response.url).href !== metadataUrl) {
- throw new Error("OAuth authorization server metadata redirects are not allowed");
- }
- return response.response.json();
- },
- ),
- filename_,
- ));
- if (!pkceMetadata) {
- debug("invalid OPDS PKCE authorization server metadata");
- return;
- }
pkceTransaction = tryCatchSync(
() => createOpdsPkceTransaction({
- application: OPDS_AUTH_APPLICATION,
- applicationVersion: _APP_VERSION,
- authenticationDocumentId: authParsed.id || undefined,
- authorizationUrl: pkceMetadata.authorizationEndpoint,
- clientId: authParsed.pkce?.clientId,
- issuer: pkceMetadata.issuer,
- redirectUri: authParsed.pkce?.redirectUri,
- scope: authParsed.pkce?.scope,
- tokenUrl: pkceMetadata.tokenEndpoint,
+ allowInsecureLoopback: ENABLE_DEV_TOOLS,
+ authorizationUrl: authParsed.links?.authenticate?.url || "",
+ tokenUrl: authParsed.links?.refresh?.url || "",
}),
filename_,
);
if (!pkceTransaction) {
- debug("invalid OPDS PKCE authentication configuration");
+ debug("invalid OPDS PKCE authenticate or refresh link");
return;
}
}
@@ -223,7 +172,7 @@ const opdsAuthFlow =
tokenType: "Bearer",
refreshUrl: pkceTransaction?.tokenUrl || authParsed?.links?.refresh?.url || undefined,
authenticateUrl: pkceTransaction?.authorizationUrl || authParsed?.links?.authenticate?.url || undefined,
- clientId: pkceTransaction?.clientId,
+ clientId: pkceTransaction ? OPDS_OAUTH_CLIENT_ID : undefined,
};
debug("authentication credential config", authCredentials);
yield* callTyped(httpSetAuthenticationToken, authCredentials);
@@ -330,7 +279,9 @@ const opdsAuthFlow =
if (err instanceof Error) {
debug("OPDS auth err", err.message);
- yield put(authActions.cancel.build());
+ if (authParsed.authenticationType === OPDS_AUTHORIZATION_CODE_PKCE_TYPE) {
+ yield put(authActions.cancel.build());
+ }
return;
} else {
@@ -644,7 +595,7 @@ async function opdsSetAuthCredentials(
headers,
});
const responseJson = await response.response?.json();
- if (!response.isSuccess && !responseJson) {
+ if (!response.isSuccess) {
throw new Error(`OAuth token endpoint failed with HTTP ${response.statusCode || 0}`);
}
return responseJson;
@@ -655,7 +606,7 @@ async function opdsSetAuthCredentials(
await httpSetAuthenticationToken({
...authCredentials,
accessToken: tokenResponse.accessToken,
- clientId: pkceTransaction.clientId,
+ clientId: OPDS_OAUTH_CLIENT_ID,
refreshToken: tokenResponse.refreshToken,
refreshUrl: pkceTransaction.tokenUrl,
tokenType,
@@ -817,15 +768,6 @@ interface IOPDSAuthDocParsed {
qop?: string,
realm?: string,
- pkce?: {
- authorizationServer?: string;
- clientId?: string;
- redirectUri?: string;
- scope?: string;
- issuer?: string;
- codeChallengeMethodsSupported?: string[];
- },
-
}
function opdsAuthDocConverter(doc: OPDSAuthenticationDoc, baseUrl: string): IOPDSAuthDocParsed | undefined {
if (!doc || !(doc instanceof OPDSAuthenticationDoc)) {
@@ -858,8 +800,7 @@ function opdsAuthDocConverter(doc: OPDSAuthenticationDoc, baseUrl: string): IOPD
return undefined;
}
- const authentication = doc.Authentication.find((v) => v.Type === OPDS_AUTHORIZATION_CODE_PKCE_TYPE) ||
- doc.Authentication.find((v) => AUTHENTICATION_TYPE.includes(v.Type as any));
+ const authentication = doc.Authentication.find((v) => AUTHENTICATION_TYPE.includes(v.Type as any));
if (!authentication) {
debug("OPDS Authentication Document does not contain a supported authentication type.");
return undefined;
@@ -951,28 +892,6 @@ function opdsAuthDocConverter(doc: OPDSAuthenticationDoc, baseUrl: string): IOPD
algorithm: typeof authentication.AdditionalJSON?.algorithm === "string" ? authentication.AdditionalJSON.algorithm : undefined,
qop: typeof authentication.AdditionalJSON?.qop === "string" ? authentication.AdditionalJSON.qop : undefined,
realm: typeof authentication.AdditionalJSON?.realm === "string" ? authentication.AdditionalJSON.realm : "", // mapping to title in opdsAuthentication json
- pkce: authentication.Type === OPDS_AUTHORIZATION_CODE_PKCE_TYPE ? {
- authorizationServer: typeof authentication.AdditionalJSON?.authorization_server === "string"
- ? authentication.AdditionalJSON.authorization_server
- : undefined,
- clientId: typeof authentication.AdditionalJSON?.client_id === "string"
- ? authentication.AdditionalJSON.client_id
- : undefined,
- redirectUri: typeof authentication.AdditionalJSON?.redirect_uri === "string"
- ? authentication.AdditionalJSON.redirect_uri
- : undefined,
- scope: typeof authentication.AdditionalJSON?.scope === "string"
- ? authentication.AdditionalJSON.scope
- : undefined,
- issuer: typeof authentication.AdditionalJSON?.issuer === "string"
- ? authentication.AdditionalJSON.issuer
- : undefined,
- codeChallengeMethodsSupported:
- Array.isArray(authentication.AdditionalJSON?.code_challenge_methods_supported)
- ? authentication.AdditionalJSON.code_challenge_methods_supported
- .filter((method): method is string => typeof method === "string")
- : undefined,
- } : undefined,
};
}
@@ -1343,6 +1262,9 @@ function parseRequestFromCustomProtocol(req: Electron.ProtocolRequest, authentic
// query component of the Redirection URI, unless a different Response Mode was specified.
if (data.error) {
debug("OAuth Error Response", "error:", { error: data.error, error_description: data.error_description });
+ if (authenticationType !== OPDS_AUTHORIZATION_CODE_PKCE_TYPE) {
+ return undefined;
+ }
}
if (authenticationType === "http://opds-spec.org/auth/oauth/implicit") {
diff --git a/test/main/network/opdsPkce.test.ts b/test/main/network/opdsPkce.test.ts
index 4ff0143213..571b0ce199 100644
--- a/test/main/network/opdsPkce.test.ts
+++ b/test/main/network/opdsPkce.test.ts
@@ -14,22 +14,21 @@ import { describe, expect, test } from "@jest/globals";
import {
IOpdsPkceCallback,
OPDS_AUTHORIZATION_CODE_PKCE_TYPE,
+ OPDS_OAUTH_CLIENT_ID,
+ OPDS_OAUTH_REDIRECT_URI,
createOpdsPkceCodeChallenge,
+ createOpdsPkceRefreshTokenRequest,
createOpdsPkceTokenRequest,
createOpdsPkceTransaction,
exchangeOpdsPkceAuthorizationCode,
getSafeOpdsAuthUrlForLog,
- loadOpdsPkceAuthorizationServerMetadata,
parseOpdsPkceTokenResponse,
validateOpdsPkceCallback,
} from "readium-desktop/main/network/opdsPkce";
import { OPDSAuthenticationDoc } from "@r2-opds-js/opds/opds2/opds2-authentication-doc";
import { TaJsonDeserialize } from "@r2-lcp-js/serializable";
-const clientId = "http://opds-spec.org/auth/client";
-const redirectUri = "opds://authorize/";
-
-describe("OPDS Authorization Code + PKCE", () => {
+describe("OPDS Authorization Code with PKCE", () => {
test("generates the RFC 7636 S256 reference challenge", () => {
const verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk";
expect(createOpdsPkceCodeChallenge(verifier)).toBe("E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM");
@@ -42,14 +41,10 @@ describe("OPDS Authorization Code + PKCE", () => {
expect(getSafeOpdsAuthUrlForLog("data:text/html,secret-content")).toBe("data:[redacted]");
});
- test("creates a public-client authorization request without exposing the verifier", () => {
+ test("creates exactly the proposed shared-client authorization request", () => {
const transaction = createOpdsPkceTransaction(
{
- authenticationDocumentId: "https://catalog.example/auth",
- authorizationUrl: "https://login.example/authorize?audience=opds",
- clientId,
- redirectUri,
- scope: "opds",
+ authorizationUrl: "https://login.example/authorize",
tokenUrl: "https://login.example/token",
},
1000,
@@ -58,85 +53,52 @@ describe("OPDS Authorization Code + PKCE", () => {
expect(transaction.codeVerifier).toMatch(/^[A-Za-z0-9_-]{43}$/);
expect(transaction.state).toMatch(/^[A-Za-z0-9_-]{43}$/);
- expect(url.searchParams.get("audience")).toBe("opds");
- expect(url.searchParams.get("response_type")).toBe("code");
- expect(url.searchParams.get("client_id")).toBe(clientId);
- expect(url.searchParams.get("redirect_uri")).toBe(redirectUri);
- expect(url.searchParams.get("code_challenge_method")).toBe("S256");
- expect(url.searchParams.get("code_challenge")).toBe(createOpdsPkceCodeChallenge(transaction.codeVerifier));
+ expect(Object.fromEntries(url.searchParams)).toEqual({
+ response_type: "code",
+ client_id: OPDS_OAUTH_CLIENT_ID,
+ redirect_uri: OPDS_OAUTH_REDIRECT_URI,
+ code_challenge: createOpdsPkceCodeChallenge(transaction.codeVerifier),
+ code_challenge_method: "S256",
+ state: transaction.state,
+ });
expect(transaction.authorizationRequestUrl).not.toContain(transaction.codeVerifier);
expect(() =>
createOpdsPkceTransaction({
authorizationUrl: "http://login.example/authorize",
- clientId,
- redirectUri,
tokenUrl: "http://login.example/token",
}),
).toThrow(/HTTPS/);
- });
-
- test("binds authorization and token endpoints to validated issuer metadata", async () => {
- const metadataUrl = "https://login.example/.well-known/oauth-authorization-server";
- const metadataDocument = {
- issuer: "https://login.example",
- authorization_endpoint: "https://login.example/authorize",
- token_endpoint: "https://login.example/token",
- authorization_response_iss_parameter_supported: true,
- code_challenge_methods_supported: ["S256"],
- };
- const configuration = {
- authorizationServerMetadataUrl: metadataUrl,
- expectedAuthorizationUrl: metadataDocument.authorization_endpoint,
- expectedIssuer: metadataDocument.issuer,
- expectedTokenUrl: metadataDocument.token_endpoint,
- };
-
- await expect(
- loadOpdsPkceAuthorizationServerMetadata(configuration, async (url) => {
- expect(url).toBe(metadataUrl);
- return metadataDocument;
+ expect(() =>
+ createOpdsPkceTransaction({
+ authorizationUrl: "http://127.0.0.1/authorize",
+ tokenUrl: "http://127.0.0.1/token",
}),
- ).resolves.toEqual({
- authorizationEndpoint: metadataDocument.authorization_endpoint,
- issuer: metadataDocument.issuer,
- tokenEndpoint: metadataDocument.token_endpoint,
- });
- await expect(
- loadOpdsPkceAuthorizationServerMetadata(
- { ...configuration, expectedTokenUrl: "https://attacker.example/token" },
- async () => metadataDocument,
- ),
- ).rejects.toThrow(/does not match/);
- await expect(
- loadOpdsPkceAuthorizationServerMetadata(
- { ...configuration, authorizationServerMetadataUrl: "https://attacker.example/metadata" },
- async () => metadataDocument,
- ),
- ).rejects.toThrow(/same origin/);
- await expect(
- loadOpdsPkceAuthorizationServerMetadata(configuration, async () => ({
- ...metadataDocument,
- authorization_response_iss_parameter_supported: false,
- })),
- ).rejects.toThrow(/iss parameter/);
+ ).toThrow(/HTTPS/);
+ expect(() =>
+ createOpdsPkceTransaction({
+ allowInsecureLoopback: true,
+ authorizationUrl: "http://127.0.0.1/authorize",
+ tokenUrl: "http://127.0.0.1/token",
+ }),
+ ).not.toThrow();
+ expect(() =>
+ createOpdsPkceTransaction({
+ authorizationUrl: "https://login.example/authorize",
+ tokenUrl: "",
+ }),
+ ).toThrow(/links are required/);
});
- test("validates state, document identity, issuer, errors, and expiry", () => {
+ test("validates callback state, errors, code, and transaction age", () => {
const transaction = createOpdsPkceTransaction(
{
- authenticationDocumentId: "https://catalog.example/auth",
authorizationUrl: "https://login.example/authorize",
- clientId,
- issuer: "https://login.example",
- redirectUri,
tokenUrl: "https://login.example/token",
},
1000,
);
const validCallback: IOpdsPkceCallback = {
code: "authorization-code",
- id: transaction.authenticationDocumentId,
- iss: transaction.issuer,
state: transaction.state,
};
@@ -144,18 +106,13 @@ describe("OPDS Authorization Code + PKCE", () => {
expect(() => validateOpdsPkceCallback({ ...validCallback, state: "wrong" }, transaction, 2000)).toThrow(
/state/,
);
- expect(() =>
- validateOpdsPkceCallback({ ...validCallback, id: "https://other.example/auth" }, transaction, 2000),
- ).toThrow(/identifier/);
- expect(() =>
- validateOpdsPkceCallback({ ...validCallback, iss: "https://other.example" }, transaction, 2000),
- ).toThrow(/issuer/);
- expect(() => validateOpdsPkceCallback({ ...validCallback, iss: undefined }, transaction, 2000)).toThrow(
- /expected issuer/,
- );
+ expect(() => validateOpdsPkceCallback({ state: transaction.state }, transaction, 2000)).toThrow(/code/);
expect(() =>
validateOpdsPkceCallback(
- { error: "access_denied", iss: transaction.issuer, state: transaction.state },
+ {
+ error: "access_denied",
+ state: transaction.state,
+ },
transaction,
2000,
),
@@ -163,20 +120,20 @@ describe("OPDS Authorization Code + PKCE", () => {
expect(() => validateOpdsPkceCallback(validCallback, transaction, 5 * 60 * 1000 + 1001)).toThrow(/expired/);
});
- test("builds the authorization-code token request and parses the token response", () => {
+ test("builds the proposed token request and parses the token response", () => {
const transaction = createOpdsPkceTransaction({
authorizationUrl: "https://login.example/authorize",
- clientId,
- redirectUri,
tokenUrl: "https://login.example/token",
});
const request = new URLSearchParams(createOpdsPkceTokenRequest(transaction, "code-value"));
- expect(request.get("grant_type")).toBe("authorization_code");
- expect(request.get("client_id")).toBe(clientId);
- expect(request.get("redirect_uri")).toBe(redirectUri);
- expect(request.get("code")).toBe("code-value");
- expect(request.get("code_verifier")).toBe(transaction.codeVerifier);
+ expect(Object.fromEntries(request)).toEqual({
+ grant_type: "authorization_code",
+ code: "code-value",
+ redirect_uri: OPDS_OAUTH_REDIRECT_URI,
+ client_id: OPDS_OAUTH_CLIENT_ID,
+ code_verifier: transaction.codeVerifier,
+ });
expect(request.has("client_secret")).toBe(false);
expect(
parseOpdsPkceTokenResponse({
@@ -187,21 +144,20 @@ describe("OPDS Authorization Code + PKCE", () => {
}),
).toEqual({
accessToken: "access-token",
- expiresIn: 600,
refreshToken: "refresh-token",
- scope: undefined,
tokenType: "bearer",
});
+ expect(Object.fromEntries(new URLSearchParams(createOpdsPkceRefreshTokenRequest("refresh-token")))).toEqual({
+ grant_type: "refresh_token",
+ refresh_token: "refresh-token",
+ client_id: OPDS_OAUTH_CLIENT_ID,
+ });
expect(() => parseOpdsPkceTokenResponse({ error: "invalid_grant" })).toThrow(/invalid_grant/);
});
- test("completes the flow against the real local authorization server", async () => {
+ test("completes the minimal flow against the local test server", async () => {
const serverPath = resolve(__dirname, "../../../projects/opds-pkce-test-server/server.mjs");
const server = spawn(process.execPath, [serverPath, "0"], {
- env: {
- ...process.env,
- OPDS_PKCE_REDIRECT_URI: redirectUri,
- },
stdio: ["ignore", "pipe", "pipe"],
});
@@ -213,51 +169,25 @@ describe("OPDS Authorization Code + PKCE", () => {
id: string;
authentication: Array<{
type: string;
- client_id: string;
- authorization_server: string;
- redirect_uri: string;
- issuer: string;
- scope: string;
links: Array<{ rel: string; href: string }>;
}>;
};
- const parsedAuthenticationDocument = TaJsonDeserialize(authenticationDocument, OPDSAuthenticationDoc);
- expect(parsedAuthenticationDocument.Authentication[0].AdditionalJSON.client_id).toBe(clientId);
- expect(
- parsedAuthenticationDocument.Authentication[0].Links.some((link) => link.Rel.includes("token")),
- ).toBe(true);
+ const parsedDocument = TaJsonDeserialize(authenticationDocument, OPDSAuthenticationDoc);
+ expect(parsedDocument.Authentication[0].Type).toBe(OPDS_AUTHORIZATION_CODE_PKCE_TYPE);
+ expect(Object.keys(authenticationDocument.authentication[0]).sort()).toEqual(["links", "type"]);
+
const authentication = authenticationDocument.authentication[0];
- expect(authentication.type).toBe(OPDS_AUTHORIZATION_CODE_PKCE_TYPE);
const authorizationUrl = authentication.links.find((link) => link.rel === "authenticate")?.href;
- const tokenUrl = authentication.links.find((link) => link.rel === "token")?.href;
+ const tokenUrl = authentication.links.find((link) => link.rel === "refresh")?.href;
expect(authorizationUrl).toBeDefined();
expect(tokenUrl).toBeDefined();
- const metadata = await loadOpdsPkceAuthorizationServerMetadata(
- {
- authorizationServerMetadataUrl: authentication.authorization_server,
- expectedAuthorizationUrl: authorizationUrl,
- expectedIssuer: authentication.issuer,
- expectedTokenUrl: tokenUrl,
- },
- async (metadataUrl) => {
- const response = await fetch(metadataUrl);
- expect(response.status).toBe(200);
- return response.json();
- },
- );
-
const transaction = createOpdsPkceTransaction({
- authenticationDocumentId: authenticationDocument.id,
- authorizationUrl: metadata.authorizationEndpoint,
- clientId: authentication.client_id,
- issuer: metadata.issuer,
- redirectUri: authentication.redirect_uri,
- scope: authentication.scope,
- tokenUrl: metadata.tokenEndpoint,
+ allowInsecureLoopback: true,
+ authorizationUrl: authorizationUrl || "",
+ tokenUrl: tokenUrl || "",
});
- const authorizationPage = await fetch(transaction.authorizationRequestUrl);
- expect(authorizationPage.status).toBe(200);
+ expect((await fetch(transaction.authorizationRequestUrl)).status).toBe(200);
const authorizationParams = new URL(transaction.authorizationRequestUrl).searchParams;
authorizationParams.set("decision", "approve");
@@ -268,29 +198,24 @@ describe("OPDS Authorization Code + PKCE", () => {
});
expect(authorizationResponse.status).toBe(303);
const callbackUrl = new URL(authorizationResponse.headers.get("location"));
- expect(callbackUrl.searchParams.get("iss")).toBe(metadata.issuer);
+ expect([...callbackUrl.searchParams.keys()].sort()).toEqual(["code", "state"]);
+
const callback = Object.fromEntries(callbackUrl.searchParams) as IOpdsPkceCallback;
const token = await exchangeOpdsPkceAuthorizationCode(transaction, callback, async (url, body) => {
const response = await fetch(url, {
body,
- headers: {
- "Content-Type": "application/x-www-form-urlencoded",
- },
+ headers: { "Content-Type": "application/x-www-form-urlencoded" },
method: "POST",
});
return response.json();
});
-
expect(token.accessToken).toBeTruthy();
expect(token.refreshToken).toBeTruthy();
+
const catalogResponse = await fetch(`${origin}/opds/v2/catalog`, {
- headers: {
- Authorization: `Bearer ${token.accessToken}`,
- },
+ headers: { Authorization: `Bearer ${token.accessToken}` },
});
expect(catalogResponse.status).toBe(200);
- const catalog = (await catalogResponse.json()) as { metadata: { title: string } };
- expect(catalog.metadata.title).toBe("Thorium PKCE Test Catalog");
} finally {
await stopServer(server);
}
From 42d273d22748c9c4462055c8f6e564bc6d9b3026 Mon Sep 17 00:00:00 2001
From: Pierre Leroux
Date: Thu, 24 Sep 2026 17:38:51 +0200
Subject: [PATCH 5/7] Replace sanitized OPDS auth URL logging with truncated
URLs
---
src/main/network/opdsPkce.ts | 12 -----
src/main/redux/sagas/auth.ts | 75 ++++++++----------------------
test/main/network/opdsPkce.test.ts | 8 ----
3 files changed, 19 insertions(+), 76 deletions(-)
diff --git a/src/main/network/opdsPkce.ts b/src/main/network/opdsPkce.ts
index 5d9b8d734e..c98483da25 100644
--- a/src/main/network/opdsPkce.ts
+++ b/src/main/network/opdsPkce.ts
@@ -56,18 +56,6 @@ export interface IOpdsPkceTokenResponse {
export type TOpdsPkceTokenPost = (url: string, body: string) => Promise;
-export function getSafeOpdsAuthUrlForLog(value: string): string {
- try {
- const url = new URL(value);
- if (url.protocol === "data:") {
- return "data:[redacted]";
- }
- return `${url.protocol}//${url.host}${url.pathname}`;
- } catch {
- return "[invalid URL]";
- }
-}
-
export function createOpdsPkceCodeChallenge(codeVerifier: string): string {
if (!PKCE_VERIFIER_REGEXP.test(codeVerifier)) {
throw new Error("The PKCE code verifier must contain 43 to 128 RFC 7636 unreserved characters.");
diff --git a/src/main/redux/sagas/auth.ts b/src/main/redux/sagas/auth.ts
index 9c09b290b3..4eb563412b 100644
--- a/src/main/redux/sagas/auth.ts
+++ b/src/main/redux/sagas/auth.ts
@@ -45,7 +45,6 @@ import {
OPDS_OAUTH_CLIENT_ID,
createOpdsPkceTransaction,
exchangeOpdsPkceAuthorizationCode,
- getSafeOpdsAuthUrlForLog,
} from "readium-desktop/main/network/opdsPkce";
import { tryCatch, tryCatchSync } from "readium-desktop/utils/tryCatch";
// eslint-disable-next-line local-rules/typed-redux-saga-use-typed-effects
@@ -164,7 +163,7 @@ const opdsAuthFlow =
debug("no valid authentication html url");
return;
}
- debug("Browser URL", getSafeOpdsAuthUrlForLog(browserUrl));
+ debug("Browser URL", browserUrl.slice(0, 100)+"...");
const authCredentials: IOpdsAuthenticationToken = {
id: authParsed?.id || undefined,
@@ -233,7 +232,7 @@ const opdsAuthFlow =
return;
}
if (!retryWithInternalBrowserWindowInsteadOfDefaultExternalWebBrowser && opdsCustomProtocolRequestParsed.data[URL_OPDS_AUTH_RETRY] === URL_OPDS_AUTH_RETRY) {
- debug("OPDS auth retry ...", getSafeOpdsAuthUrlForLog(opdsCustomProtocolRequestParsed.url.href));
+ debug("OPDS auth retry ...", opdsCustomProtocolRequestParsed.url);
callback({
url: undefined,
@@ -252,12 +251,7 @@ const opdsAuthFlow =
// yield put(historyActions.refresh.build()); // ==> keep current context and recalls auth, but we need retryWithInternalBrowserWindowInsteadOfDefaultExternalWebBrowser
yield spawn(function* () {
- debug(
- "OPDS auth retry GO!",
- getSafeOpdsAuthUrlForLog(opdsCustomProtocolRequestParsed.url.href),
- getSafeOpdsAuthUrlForLog(baseUrl),
- JSON.stringify(doc, null, 4),
- );
+ debug("OPDS auth retry GO!", opdsCustomProtocolRequestParsed.url, baseUrl, JSON.stringify(doc, null, 4));
const opdsAuthChannel = getOpdsAuthenticationChannel();
opdsAuthChannel.put([doc, baseUrl, true]); // retryWithInternalBrowserWindowInsteadOfDefaultExternalWebBrowser
});
@@ -897,7 +891,7 @@ function opdsAuthDocConverter(doc: OPDSAuthenticationDoc, baseUrl: string): IOPD
async function createOpdsAuthenticationModalWin(urlStr: string, retryWithInternalBrowserWindowInsteadOfDefaultExternalWebBrowser: boolean): Promise {
- debug("OPDS AUTH win URL", getSafeOpdsAuthUrlForLog(urlStr));
+ debug("OPDS AUTH win URL", urlStr.slice(0, 100) + (urlStr.length > 100 ? "..." : ""));
const libWin = tryCatchSync(() => getLibraryWindowFromDi(), filename_);
if (!libWin || libWin.isDestroyed() || libWin.webContents.isDestroyed()) {
@@ -911,7 +905,7 @@ async function createOpdsAuthenticationModalWin(urlStr: string, retryWithInterna
// passthrough
} else if (/^https?:\/\//.test(urlStr)) {
if (!retryWithInternalBrowserWindowInsteadOfDefaultExternalWebBrowser) {
- debug("OPDS AUTH win URL EXTERNAL ...", getSafeOpdsAuthUrlForLog(urlStr));
+ debug("OPDS AUTH win URL EXTERNAL ...", urlStr);
urlExternal = urlStr;
title = getTranslator().translate("catalog.opds.auth.login");
@@ -930,7 +924,7 @@ async function createOpdsAuthenticationModalWin(urlStr: string, retryWithInterna
// return undefined;
}
} else {
- debug("INVALID AUTH urlStr", getSafeOpdsAuthUrlForLog(urlStr));
+ debug("INVALID AUTH urlStr", urlStr);
return undefined;
}
@@ -979,7 +973,7 @@ async function createOpdsAuthenticationModalWin(urlStr: string, retryWithInterna
// });
win.once("ready-to-show", () => {
- debug("OPDS AUTH win ready-to-show", getSafeOpdsAuthUrlForLog(urlStr));
+ debug("OPDS AUTH win ready-to-show", urlStr.substring(0, 500));
win.show();
});
@@ -992,28 +986,16 @@ async function createOpdsAuthenticationModalWin(urlStr: string, retryWithInterna
if (/^https?:\/\//.test(navUrl)) { // ignores file: mailto: data: thoriumhttps: httpsr2: thorium: opds: etc.
- debug(
- "willNavigate ==> EXTERNAL: ",
- getSafeOpdsAuthUrlForLog(win.webContents.getURL()),
- " *** ",
- getSafeOpdsAuthUrlForLog(navUrl),
- );
+ debug("willNavigate ==> EXTERNAL: ", win.webContents.getURL().substring(0, 500), " *** ", navUrl);
shell.openExternal(navUrl).then(() => { /* noop */ }).catch((err: unknown) => { debug(err); }); // .finally(() => { /* noop */ })
return;
}
- debug("willNavigate ==> noop: ", getSafeOpdsAuthUrlForLog(navUrl));
+ debug("willNavigate ==> noop: ", navUrl);
};
win.webContents.setWindowOpenHandler((details: HandlerDetails) => {
- debug(
- "BrowserWindow.webContents.setWindowOpenHandler (always DENY), win.webContents.id: ",
- win.webContents.id,
- "\n --- details.url: ",
- getSafeOpdsAuthUrlForLog(details.url),
- "\n === win.webContents.getURL()",
- getSafeOpdsAuthUrlForLog(win.webContents.getURL()),
- );
+ debug("BrowserWindow.webContents.setWindowOpenHandler (always DENY), win.webContents.id: ", win.webContents.id, "\n --- details.url: ", details.url.substring(0, 500), "\n === win.webContents.getURL()", win.webContents.getURL().substring(0, 500));
// willNavigate(details.url);
@@ -1021,28 +1003,14 @@ async function createOpdsAuthenticationModalWin(urlStr: string, retryWithInterna
});
win.webContents.on("will-navigate", (details: ElectronEvent, detailsUrl: string) => {
- debug(
- "BrowserWindow.webContents.on('will-navigate') (always PREVENT?), win.webContents.id: ",
- win.webContents.id,
- "\n --- details.url: ",
- details.url ? getSafeOpdsAuthUrlForLog(details.url) : undefined,
- "\n *** detailsUrl: ",
- detailsUrl ? getSafeOpdsAuthUrlForLog(detailsUrl) : undefined,
- "\n ~~~ urlStr: ",
- getSafeOpdsAuthUrlForLog(urlStr),
- "\n === win.webContents.getURL(): ",
- getSafeOpdsAuthUrlForLog(win.webContents.getURL()),
- );
+ debug("BrowserWindow.webContents.on('will-navigate') (always PREVENT?), win.webContents.id: ", win.webContents.id, "\n --- details.url: ", details.url?.substring(0, 500), "\n *** detailsUrl: ", detailsUrl?.substring(0, 500), "\n ~~~ urlStr: ", urlStr.substring(0, 500), "\n === win.webContents.getURL(): ", win.webContents.getURL()?.substring(0, 500));
if (details.url?.startsWith(`${URL_PROTOCOL_OPDS}://${URL_HOST_OPDS_AUTH}/`)) {
- debug(
- `${URL_PROTOCOL_OPDS}://${URL_HOST_OPDS_AUTH}/ ==> PASS: `,
- getSafeOpdsAuthUrlForLog(details.url),
- );
+ debug(`${URL_PROTOCOL_OPDS}://${URL_HOST_OPDS_AUTH}/ ==> PASS: `, details.url?.substring(0, 500));
return;
}
if (details.url === win.webContents.getURL()) {
- debug("same URL ==> PASS: ", getSafeOpdsAuthUrlForLog(details.url));
+ debug("same URL ==> PASS: ", details.url?.substring(0, 500));
return;
}
@@ -1131,15 +1099,13 @@ async function createOpdsAuthenticationModalWin(urlStr: string, retryWithInterna
// });
// });
- debug("OPDS AUTH win LOAD 1", getSafeOpdsAuthUrlForLog(urlStr));
+ debug("OPDS AUTH win LOAD 1", urlStr.substring(0, 500));
// win.webContents.loadURL
// await DO NOT AWAIT!! (race condition when urlStr is a HTTP link that immediately redirects to OPDS://AUTHORIZE)
- win.loadURL(urlStr)
- .then(() => { debug("loadURL() ok", getSafeOpdsAuthUrlForLog(urlStr)); })
- .catch((err) => { debug("loadURL() nok", getSafeOpdsAuthUrlForLog(urlStr)); debug(err); });
+ win.loadURL(urlStr).then(() => { debug("loadURL() ok " + urlStr); }).catch((err) => { debug("loadURL() nok " + urlStr); debug(err); });
- debug("OPDS AUTH win LOAD 2", getSafeOpdsAuthUrlForLog(urlStr));
+ debug("OPDS AUTH win LOAD 2", urlStr.substring(0, 500));
if (urlExternal) {
setTimeout(() => {
@@ -1160,12 +1126,9 @@ interface IParseRequestFromCustomProtocol {
function parseRequestFromCustomProtocol(req: Electron.ProtocolRequest, authenticationType: TAuthenticationType)
: IParseRequestFromCustomProtocol | undefined {
- debug("opds:// request:", {
- method: typeof req === "object" ? req.method : undefined,
- url: typeof req === "object" && typeof req.url === "string"
- ? getSafeOpdsAuthUrlForLog(req.url)
- : undefined,
- });
+ debug("########");
+ debug("opds:// request:", req);
+ debug("########");
if (typeof req === "object") {
const { method, url, uploadData } = req;
diff --git a/test/main/network/opdsPkce.test.ts b/test/main/network/opdsPkce.test.ts
index 571b0ce199..80d68973fb 100644
--- a/test/main/network/opdsPkce.test.ts
+++ b/test/main/network/opdsPkce.test.ts
@@ -21,7 +21,6 @@ import {
createOpdsPkceTokenRequest,
createOpdsPkceTransaction,
exchangeOpdsPkceAuthorizationCode,
- getSafeOpdsAuthUrlForLog,
parseOpdsPkceTokenResponse,
validateOpdsPkceCallback,
} from "readium-desktop/main/network/opdsPkce";
@@ -34,13 +33,6 @@ describe("OPDS Authorization Code with PKCE", () => {
expect(createOpdsPkceCodeChallenge(verifier)).toBe("E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM");
});
- test("redacts OAuth parameters from logged URLs", () => {
- expect(
- getSafeOpdsAuthUrlForLog("opds://authorize/?code=authorization-code&state=state-value#access_token=token"),
- ).toBe("opds://authorize/");
- expect(getSafeOpdsAuthUrlForLog("data:text/html,secret-content")).toBe("data:[redacted]");
- });
-
test("creates exactly the proposed shared-client authorization request", () => {
const transaction = createOpdsPkceTransaction(
{
From 8f2b8e131db50472fddcbec5dafea1d375cb0d6e Mon Sep 17 00:00:00 2001
From: Pierre Leroux
Date: Thu, 24 Sep 2026 17:57:35 +0200
Subject: [PATCH 6/7] up
---
src/main/network/http.ts | 8 +++-----
src/main/redux/sagas/auth.ts | 4 ++--
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/src/main/network/http.ts b/src/main/network/http.ts
index 148962ac53..55dd0be2b1 100644
--- a/src/main/network/http.ts
+++ b/src/main/network/http.ts
@@ -93,7 +93,7 @@ export interface IOpdsAuthenticationToken {
opdsAuthenticationUrl?: string; // application/opds-authentication+json
refreshUrl?: string;
authenticateUrl?: string;
- clientId?: string;
+ pkce?: boolean;
accessToken?: string;
refreshToken?: string;
tokenType?: string;
@@ -683,11 +683,9 @@ const httpGetUnauthorizedRefresh =
options.headers = options.headers instanceof Headers
? options.headers
: new Headers(options.headers || {});
- // PKCE credentials include a public client ID and use the standard OAuth form-encoded
- // refresh request. Keep the JSON body for legacy OPDS credentials that predate clientId.
- if (auth.clientId) {
+ if (auth.pkce) {
(options.headers as Headers).set("Content-Type", "application/x-www-form-urlencoded");
- options.body = createOpdsPkceRefreshTokenRequest(refreshToken, auth.clientId);
+ options.body = createOpdsPkceRefreshTokenRequest(refreshToken);
} else {
(options.headers as Headers).set("Content-Type", "application/json");
options.body = JSON.stringify({
diff --git a/src/main/redux/sagas/auth.ts b/src/main/redux/sagas/auth.ts
index 4eb563412b..a6f3e487f4 100644
--- a/src/main/redux/sagas/auth.ts
+++ b/src/main/redux/sagas/auth.ts
@@ -171,7 +171,7 @@ const opdsAuthFlow =
tokenType: "Bearer",
refreshUrl: pkceTransaction?.tokenUrl || authParsed?.links?.refresh?.url || undefined,
authenticateUrl: pkceTransaction?.authorizationUrl || authParsed?.links?.authenticate?.url || undefined,
- clientId: pkceTransaction ? OPDS_OAUTH_CLIENT_ID : undefined,
+ pkce: !!pkceTransaction,
};
debug("authentication credential config", authCredentials);
yield* callTyped(httpSetAuthenticationToken, authCredentials);
@@ -600,7 +600,7 @@ async function opdsSetAuthCredentials(
await httpSetAuthenticationToken({
...authCredentials,
accessToken: tokenResponse.accessToken,
- clientId: OPDS_OAUTH_CLIENT_ID,
+ pkce: true,
refreshToken: tokenResponse.refreshToken,
refreshUrl: pkceTransaction.tokenUrl,
tokenType,
From 1a42c8f0694e601d5d26c542c2be2b339476e219 Mon Sep 17 00:00:00 2001
From: Pierre Leroux
Date: Thu, 24 Sep 2026 18:11:54 +0200
Subject: [PATCH 7/7] lint
---
src/main/redux/sagas/auth.ts | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/main/redux/sagas/auth.ts b/src/main/redux/sagas/auth.ts
index a6f3e487f4..70c521911b 100644
--- a/src/main/redux/sagas/auth.ts
+++ b/src/main/redux/sagas/auth.ts
@@ -42,7 +42,6 @@ import { ContentType } from "readium-desktop/utils/contentType";
import {
IOpdsPkceTransaction,
OPDS_AUTHORIZATION_CODE_PKCE_TYPE,
- OPDS_OAUTH_CLIENT_ID,
createOpdsPkceTransaction,
exchangeOpdsPkceAuthorizationCode,
} from "readium-desktop/main/network/opdsPkce";