281 lines
11 KiB
Markdown
281 lines
11 KiB
Markdown
# Research: OIDC Proxy Script Authentication
|
||
|
||
**Feature**: 001-oidc-proxy-script
|
||
**Phase**: 0 — Resolved unknowns
|
||
**Date**: 2025-07-17
|
||
|
||
---
|
||
|
||
## R-001 — Token Cache Persistence in VM Sandbox
|
||
|
||
**Unknown**: How can `proxy.js` maintain a token cache that survives across requests when
|
||
`server.js` creates a fresh `vm.createContext()` per request (resetting all bare `let` variables)?
|
||
|
||
**Decision**: Store the cache as a property on the `adapter_settings` object
|
||
(`adapter_settings._cache`).
|
||
|
||
**Rationale**: `server.js` loads `adapter_settings.json` into `globalVariableContext.adapter_settings`
|
||
once at startup. The per-request spread `vm.createContext({ ...globalVariableContext })` copies the
|
||
*object reference* — not a clone — into each sandbox. Any mutation to `adapter_settings._cache`
|
||
modifies the same underlying heap object and is therefore visible to every subsequent request.
|
||
Bare `let`/`const` variables at the top level of `proxy.js` do **not** persist; they are
|
||
sandbox-local and are reset to `undefined` on every invocation.
|
||
|
||
```javascript
|
||
// proxy.js — top of script (safe, no import/export)
|
||
// adapter_settings is the same JS object reference every invocation:
|
||
const _cache = adapter_settings._cache ||
|
||
(adapter_settings._cache = { token: null, expiry: 0, pendingFetch: null });
|
||
```
|
||
|
||
**Alternatives considered**:
|
||
|
||
| Alternative | Why rejected |
|
||
|-------------|-------------|
|
||
| Bare `let cachedToken` in proxy.js | Resets on every `vm.createContext()` invocation |
|
||
| Add dedicated `_cache` to `globalVariableContext` in server.js | Correct but requires server.js modification; feature spec says no server.js changes needed |
|
||
| External Redis / file cache | Introduces infrastructure dependency; spec assumption explicitly rules this out |
|
||
| Attach to `axios` or another injected object | Correct mechanism but semantically wrong; `adapter_settings` is the natural owner |
|
||
|
||
**Verification**: `globalVariableContext` is a module-level `let` in `server.js` (line 27).
|
||
The spread in `vm.createContext` (lines 172–177) copies each property value by reference for
|
||
objects. `adapter_settings` is an object, so the sandbox and `globalVariableContext` share the
|
||
same reference. Confirmed by reading `src/server.js`.
|
||
|
||
---
|
||
|
||
## R-002 — Token Stampede Prevention (Promise Sharing)
|
||
|
||
**Unknown**: When multiple concurrent requests arrive while `_cache.token` is null, how do we
|
||
ensure only one HTTP request is sent to the token service?
|
||
|
||
**Decision**: Store the in-flight fetch promise on `_cache.pendingFetch`. Subsequent requests
|
||
detect a non-null `pendingFetch` (via duck-type check) and `await` the same promise. A `finally`
|
||
block clears `pendingFetch` after settlement.
|
||
|
||
**Rationale**: The `pendingFetch` property is on the shared `adapter_settings._cache` object
|
||
(same reference as R-001). All concurrent requests therefore see the same pending Promise.
|
||
|
||
**Cross-realm safety**: Each `vm.createContext()` creates a new V8 realm with its own
|
||
`Promise` constructor. `instanceof Promise` checks fail across realms. The Promises/A+ thenable
|
||
protocol (`await` and `.then()`) works via duck-typing and is cross-realm safe.
|
||
|
||
```javascript
|
||
// Stampede guard — duck-type check, NOT instanceof
|
||
if (_cache.pendingFetch !== null &&
|
||
typeof _cache.pendingFetch.then === 'function') {
|
||
// Queue on the existing fetch — await is thenable-protocol safe across V8 realms
|
||
try {
|
||
await _cache.pendingFetch;
|
||
res.writeHead(200, { 'Content-Type': 'text/plain' });
|
||
res.end('Authorized');
|
||
} catch (err) {
|
||
res.writeHead(401, { 'Content-Type': 'text/plain' });
|
||
res.end('Unauthorized: ' + err.message);
|
||
}
|
||
return;
|
||
}
|
||
|
||
// This invocation wins the race — build and share the fetch promise
|
||
_cache.pendingFetch = (async () => {
|
||
// ... axios.post ...
|
||
_cache.token = id_token;
|
||
_cache.expiry = expires_in; // absolute Unix epoch seconds (FR-006)
|
||
})();
|
||
|
||
try {
|
||
await _cache.pendingFetch;
|
||
res.writeHead(200, { 'Content-Type': 'text/plain' });
|
||
res.end('Authorized');
|
||
} catch (err) {
|
||
_cache.token = null;
|
||
_cache.expiry = 0;
|
||
res.writeHead(401, { 'Content-Type': 'text/plain' });
|
||
res.end('Unauthorized: ' + err.message);
|
||
} finally {
|
||
_cache.pendingFetch = null; // clear after all awaiters have resolved/rejected
|
||
}
|
||
```
|
||
|
||
**Why `finally` is safe for clearing `pendingFetch`**: By the time `finally` runs, all callers
|
||
that `await`-ed `_cache.pendingFetch` have already received the settled value. Setting
|
||
`pendingFetch = null` only affects future requests arriving after settlement.
|
||
|
||
**Alternatives considered**:
|
||
|
||
| Alternative | Why rejected |
|
||
|-------------|-------------|
|
||
| `instanceof Promise` guard | Fails across V8 realms — each sandbox has its own `Promise` constructor |
|
||
| Mutex / lock via integer flag | More complex; promise sharing is idiomatic in async JS |
|
||
| Separate request queue (array + callbacks) | Overkill; promise sharing achieves the same result with fewer lines |
|
||
|
||
---
|
||
|
||
## R-003 — Async Proxy Script in Synchronous `runInContext`
|
||
|
||
**Unknown**: `server.js` calls `script.runInContext(context)` synchronously without `await`.
|
||
How can `proxy.js` do async work (HTTP call) without leaving unhandled promise rejections?
|
||
|
||
**Decision**: Wrap all proxy logic in a top-level immediately-invoked async function expression
|
||
(IIFE). Catch all errors inside the IIFE and always send a response before the IIFE resolves.
|
||
|
||
**Rationale**: The async IIFE returns a Promise but `server.js` does not `await` it. The
|
||
outer `try/catch` in `server.js` only catches synchronous throws. All async errors must be
|
||
handled within `proxy.js` itself.
|
||
|
||
```javascript
|
||
// proxy.js — entire body wrapped in async IIFE
|
||
(async () => {
|
||
try {
|
||
// ... token logic ...
|
||
res.writeHead(200, { 'Content-Type': 'text/plain' });
|
||
res.end('Authorized');
|
||
} catch (err) {
|
||
console.error({ message: 'Auth failed', error: err.message });
|
||
res.writeHead(401, { 'Content-Type': 'text/plain' });
|
||
res.end('Unauthorized: ' + err.message);
|
||
}
|
||
})();
|
||
```
|
||
|
||
**Constraint**: `res.end()` MUST be called in both the `try` and `catch` paths. If `res.end()`
|
||
is not called, the HTTP connection hangs for the caller.
|
||
|
||
---
|
||
|
||
## R-004 — Absolute Epoch Expiry Check
|
||
|
||
**Unknown**: The spec states `expires_in` is an absolute Unix epoch timestamp (not a
|
||
relative duration). What is the correct expiry check?
|
||
|
||
**Decision**: `Date.now() / 1000 < _cache.expiry` — token is valid when current Unix time
|
||
(seconds) is strictly less than the stored `expires_in` value.
|
||
|
||
**Rationale**: Confirmed in spec (line 99): "Expiry check: `Date.now() / 1000 < expires_in`".
|
||
Example value `1532618185` is a Unix timestamp, not a duration.
|
||
|
||
```javascript
|
||
function isTokenValid() {
|
||
return _cache.token !== null && Date.now() / 1000 < _cache.expiry;
|
||
}
|
||
```
|
||
|
||
**Edge cases**:
|
||
- `expires_in` already in the past on receipt → `isTokenValid()` returns `false` immediately →
|
||
fresh token fetched (FR-006 compliance)
|
||
- `expires_in` is `0` or negative → treated as expired
|
||
- No safety buffer is applied; the spec does not require one
|
||
|
||
---
|
||
|
||
## R-005 — axios OIDC POST Pattern
|
||
|
||
**Decision**: Pass `URLSearchParams` instance as body; set `Content-Type` header explicitly for
|
||
clarity; use `timeout: 5000` for the 5-second limit (FR-014).
|
||
|
||
```javascript
|
||
const params = new URLSearchParams({
|
||
grant_type: 'password',
|
||
username: adapter_settings.username,
|
||
password: adapter_settings.password,
|
||
client_id: adapter_settings.clientId,
|
||
scope: adapter_settings.scope,
|
||
});
|
||
|
||
const response = await axios.post(adapter_settings.tokenUrl, params, {
|
||
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
|
||
timeout: 5000,
|
||
});
|
||
```
|
||
|
||
**Content-Type note**: axios v1.x auto-detects `URLSearchParams` and sets
|
||
`application/x-www-form-urlencoded` automatically. The explicit header is belt-and-suspenders
|
||
for readability in the VM sandbox context.
|
||
|
||
---
|
||
|
||
## R-006 — axios Error Differentiation
|
||
|
||
**Decision**: Treat **all** axios errors as authentication failures (FR-008). Derive the
|
||
error message from the error type for clarity in the 401 body:
|
||
|
||
| Error condition | `error.response` | `error.code` | Message strategy |
|
||
|-----------------|-----------------|--------------|-----------------|
|
||
| HTTP 4xx/5xx from token service | Populated | — | `HTTP ${error.response.status}` |
|
||
| Timeout (5 s) | `undefined` | `'ECONNABORTED'` / `'ERR_CANCELED'` | `'token service timeout'` |
|
||
| Network failure (DNS, TCP) | `undefined` | `'ERR_NETWORK'` | `error.message` |
|
||
| Missing `id_token` in response | — | — | `'id_token missing from response'` |
|
||
| Missing `adapter_settings` fields | — | — | `'missing required field: <name>'` |
|
||
|
||
All cases route to the same 401 response path, satisfying FR-008 and SC-004.
|
||
|
||
---
|
||
|
||
## R-007 — Testing VM-Sandboxed Code with `node:test`
|
||
|
||
**Decision**: Two test layers, no external test framework:
|
||
|
||
1. **Unit tests** (`tests/unit/proxy.test.js`): Compile `proxy.js` once with `new vm.Script()`.
|
||
Each test creates a controlled fake context (mock `axios`, controllable `_cache`, mock `res`).
|
||
Use `await script.runInContext(ctx)` to drive the async IIFE.
|
||
Use `t.mock.fn()` for call-count assertions; `t.mock.timers` for time-travel expiry tests.
|
||
|
||
2. **Contract tests** (`tests/contract/proxy-http.test.js`): Start an actual HTTP server with
|
||
a mock token endpoint (using `http.createServer`) and assert real HTTP responses. Validates
|
||
end-to-end behaviour including `server.js` context injection.
|
||
|
||
**Key insight**: Because `proxy.js` receives **all** dependencies via the VM context object,
|
||
dependency injection IS the test seam. No module-level mocking (`jest.mock` equivalent) is
|
||
needed or available — the context object serves the same role.
|
||
|
||
```javascript
|
||
// tests/unit/proxy.test.js — shared setup pattern
|
||
import { test, describe } from 'node:test';
|
||
import assert from 'node:assert/strict';
|
||
import vm from 'node:vm';
|
||
import { readFileSync } from 'node:fs';
|
||
import { fileURLToPath } from 'node:url';
|
||
import { dirname, join } from 'node:path';
|
||
|
||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||
const proxyCode = readFileSync(
|
||
join(__dirname, '../../src/proxyScripts/proxy.js'), 'utf-8'
|
||
);
|
||
const script = new vm.Script(proxyCode, { filename: 'proxy.js' });
|
||
|
||
function makeContext(t, overrides = {}) {
|
||
const _cache = { token: null, expiry: 0, pendingFetch: null };
|
||
let statusCode = null;
|
||
let body = '';
|
||
const res = {
|
||
writeHead: t.mock.fn((code) => { statusCode = code; }),
|
||
end: t.mock.fn((b = '') => { body += b; }),
|
||
get statusCode() { return statusCode; },
|
||
get body() { return body; },
|
||
};
|
||
return vm.createContext({
|
||
URLSearchParams,
|
||
URL,
|
||
console,
|
||
axios: {
|
||
post: t.mock.fn(async () => ({
|
||
data: { id_token: 'test-token', expires_in: 9_999_999_999 }
|
||
}))
|
||
},
|
||
adapter_settings: {
|
||
tokenUrl: 'https://auth.example.com/token',
|
||
username: 'user',
|
||
password: 'pass',
|
||
clientId: 'client',
|
||
scope: 'openid',
|
||
_cache,
|
||
},
|
||
req: { url: '/proxy', method: 'GET', headers: {} },
|
||
res,
|
||
...overrides,
|
||
});
|
||
}
|
||
```
|
||
|
||
**Run command**: `node --test tests/unit/proxy.test.js`
|