openvibers/openvibe.reviews Commits

Sign-in: the OAuth half is openvibe-sdk/sso (#215)

OP OpenVibers committed 7 days ago · c5b4eaf2bd · 3 files, +182 −45

The authorize URL, the PKCE S256 pair (new here: this copied layer had none),
the code, refresh and FedCM grants and the new GET /auth/health come from
openvibe-sdk/sso's createSsoClient. The cookies are the ones this site has
always used (ov_token, ov_refresh, ov_sso_hint, and the flow cookies on /auth
plus ov_oauth_verifier), the state is compared with the flow cookie in
constant time, next is still sanitized by this repo's sanitizer, the viewer
resolver and every permission check are untouched, and the redirect URI is
unchanged.

Changes

ddiff --git a/server/auth/session.js b/server/auth/session.js
iindex b4d84fe..b0b19b0 100644
---- a/server/auth/session.js
++++ b/server/auth/session.js
@@@ -2,27 +2,37 @@
/**
* openvibe.reviews as an OAuth2 client of OpenVibe.Network (client `reviews`, redirect
* <BASE_URL>/auth/callback) — the same session layer as OpenVibe.Community's server/auth/routes.js.
++ * The OAuth half is openvibe-sdk/sso's (createSsoClient): the authorize URL, the PKCE S256 pair, the code,
++ * refresh and FedCM grants and GET /auth/health. The state is compared with the flow cookie in constant
++ * time, next is still sanitized by this repo's sanitizer and the viewer resolver (and with it every
++ * permission check) is untouched. The redirect URI is unchanged.
*
* GET /auth/login → Network /oauth/authorize (?silent=1 adds prompt=none; ?next= same-site path)
-- * GET /auth/callback → server-side code exchange, sets the cookies
++ * GET /auth/callback → server-side code exchange with the PKCE verifier, sets the cookies
* POST /auth/fedcm → the shared navbar's FedCM assertion (nonce checked here, signature by the Network)
++ * GET /auth/health → does the Network accept this OAuth client? 503 only when it refuses it
* GET /auth/logout → clears the cookies (best-effort refresh revoke)
* GET /auth/me → the signed-in profile (offline JWT verification)
* POST /auth/refresh → rotates tokens with the refresh token
*
-- * Cookies (host-only): ov_token (access JWT, JS-readable for the navbar), ov_refresh (httpOnly,
-- * Path=/auth), ov_sso_hint ('account' | 'guest', tells the navbar whether a silent login is worth it).
++ * Cookies (host-only, the names this site has always used): ov_token (access JWT, JS-readable for the navbar),
++ * ov_refresh (httpOnly, Path=/auth), ov_sso_hint ('account' | 'guest', tells the navbar whether a silent login is
++ * worth it) and the short-lived flow cookies on /auth (ov_oauth_state, ov_oauth_next, ov_oauth_silent plus the
++ * PKCE verifier in ov_oauth_verifier).
*/
const crypto = require('crypto');
const express = require('express');
const cache = require('openvibe-shared/cache-policy');
++const { createSsoClient, failurePage } = require('openvibe-sdk/sso');
const ACCESS_COOKIE = 'ov_token';
const REFRESH_COOKIE = 'ov_refresh';
const HINT_COOKIE = 'ov_sso_hint';
const STATE_COOKIE = 'ov_oauth_state';
++const VERIFIER_COOKIE = 'ov_oauth_verifier';
const NEXT_COOKIE = 'ov_oauth_next';
const SILENT_COOKIE = 'ov_oauth_silent';
++const SITE_NAME = 'OpenVibe.Reviews';
function sanitizeNext(next, config) {
// Browsers drop tab and newline characters from a URL and read a backslash as "/": "/<TAB>/evil.com" would
@@@ -46,16 +56,11 @@ function withParam(target, key, value) {
return `${base}${base.includes('?') ? '&' : '?'}${encodeURIComponent(key)}=${encodeURIComponent(value)}${hash}`;
}
--function authorizeUrl(config, { silent = false } = {}) {
-- const state = crypto.randomBytes(16).toString('hex');
-- const u = new URL(`${config.networkUrl}/oauth/authorize`);
-- u.searchParams.set('client_id', config.oauth.clientId);
-- u.searchParams.set('redirect_uri', config.oauth.redirectUri);
-- u.searchParams.set('response_type', 'code');
-- u.searchParams.set('scope', config.oauth.scope);
-- u.searchParams.set('state', state);
-- if (silent) u.searchParams.set('prompt', 'none');
-- return { url: u.toString(), state };
++/** Compare two strings in constant time (the state must match the browser that started the flow). */
++function sameString(a, b) {
++ const x = Buffer.from(String(a || ''));
++ const y = Buffer.from(String(b || ''));
++ return x.length === y.length && x.length > 0 && crypto.timingSafeEqual(x, y);
}
function fedcmNonceMatches(token, nonce) {
@@@ -72,6 +77,24 @@ function fedcmNonceMatches(token, nonce) {
function createSessionRoutes({ config, viewers, fetchImpl = globalThis.fetch, log = console }) {
const router = express.Router();
const secure = config.cookies.secure;
++ // The OAuth half is the SDK's: the authorize URL, the PKCE S256 pair, the token grants and the health check.
++ // The session tokens are still verified by this site's own viewer resolver (server/auth/viewer.js: issuer,
++ // audience, expiry, never a service or typed token) — that part does not move.
++ const sdkSso = createSsoClient({
++ site: 'reviews',
++ siteName: SITE_NAME,
++ baseUrl: config.baseUrl,
++ clientId: config.oauth.clientId,
++ clientSecret: config.oauth.clientSecret,
++ redirectUri: config.oauth.redirectUri,
++ scope: config.oauth.scope,
++ networkUrl: config.networkUrl,
++ networkInternalUrl: config.networkInternalUrl,
++ issuer: config.networkIssuer,
++ secureCookies: secure,
++ fetch: fetchImpl,
++ log,
++ });
const access = () => ({ sameSite: 'lax', secure, httpOnly: false, path: '/', maxAge: 24 * 3600 * 1000 });
const refresh = () => ({ sameSite: 'lax', secure, httpOnly: true, path: '/auth', maxAge: 30 * 24 * 3600 * 1000 });
const flow = () => ({ sameSite: 'lax', secure, httpOnly: true, path: '/auth', maxAge: 10 * 60 * 1000 });
@@@ -86,28 +109,7 @@ function createSessionRoutes({ config, viewers, fetchImpl = globalThis.fetch, lo
res.clearCookie(ACCESS_COOKIE, { ...access(), maxAge: undefined });
res.clearCookie(REFRESH_COOKIE, { ...refresh(), maxAge: undefined });
}
-- function clearFlow(res) { for (const c of [STATE_COOKIE, NEXT_COOKIE, SILENT_COOKIE]) res.clearCookie(c, { path: '/auth' }); }
--
-- async function tokenGrant(body) {
-- let lastErr = null;
-- for (const base of [config.networkInternalUrl, config.networkUrl]) {
-- if (!base) continue;
-- try {
-- const res = await fetchImpl(`${base}/oauth/token`, {
-- method: 'POST', headers: { 'Content-Type': 'application/json' },
-- body: JSON.stringify({ client_id: config.oauth.clientId, client_secret: config.oauth.clientSecret, ...body }),
-- signal: AbortSignal.timeout(10000),
-- });
-- const data = await res.json().catch(() => ({}));
-- if (!res.ok) throw Object.assign(new Error(data.error_description || data.error || `token grant failed (${res.status})`), { status: res.status, error: data.error || 'invalid_grant' });
-- return data;
-- } catch (err) {
-- lastErr = err;
-- if (err.status && err.status < 500) throw err;
-- }
-- }
-- throw lastErr || new Error('Network unreachable');
-- }
++ function clearFlow(res) { for (const c of [STATE_COOKIE, VERIFIER_COOKIE, NEXT_COOKIE, SILENT_COOKIE]) res.clearCookie(c, { path: '/auth' }); }
router.get('/login', async (req, res) => {
const silent = !!req.query.silent && req.query.silent !== '0';
@@@ -116,11 +118,12 @@ function createSessionRoutes({ config, viewers, fetchImpl = globalThis.fetch, lo
const actor = await viewers.resolve(req, { services: false }).catch(() => null);
if (actor && actor.kind === 'user') { clearFlow(res); return res.redirect(next); }
}
-- const { url, state } = authorizeUrl(config, { silent });
-- res.cookie(STATE_COOKIE, state, flow());
++ const started = sdkSso.authorizeUrl({ silent });
++ res.cookie(STATE_COOKIE, started.state, flow());
++ res.cookie(VERIFIER_COOKIE, started.verifier, flow());
if (next !== '/') res.cookie(NEXT_COOKIE, next, flow()); else res.clearCookie(NEXT_COOKIE, { path: '/auth' });
if (silent) res.cookie(SILENT_COOKIE, '1', flow()); else res.clearCookie(SILENT_COOKIE, { path: '/auth' });
-- res.redirect(url);
++ res.redirect(started.url);
});
router.get('/callback', async (req, res) => {
@@@ -134,25 +137,43 @@ function createSessionRoutes({ config, viewers, fetchImpl = globalThis.fetch, lo
}
if (!code) return res.status(400).type('text/plain').send('Missing authorization code');
const expected = req.cookies && req.cookies[STATE_COOKIE];
++ const verifier = req.cookies && req.cookies[VERIFIER_COOKIE];
clearFlow(res);
-- if (!expected || !state || !crypto.timingSafeEqual(Buffer.from(String(state).padEnd(64).slice(0, 64)), Buffer.from(String(expected).padEnd(64).slice(0, 64)))) {
++ if (!verifier || !sameString(state, expected)) {
return res.status(400).type('text/plain').send('OAuth state mismatch — please try signing in again.');
}
try {
-- const data = await tokenGrant({ grant_type: 'authorization_code', redirect_uri: config.oauth.redirectUri, code });
++ const data = await sdkSso.tokenGrant({ grant_type: 'authorization_code', redirect_uri: config.oauth.redirectUri, code: String(code), code_verifier: verifier });
setSession(res, data.access_token, data.refresh_token);
return res.redirect(next);
} catch (err) {
++ if (err.status === 401 || err.error === 'invalid_client' || err.error === 'unauthorized_client') {
++ log.error(`[Reviews] the Network refuses this site's OAuth client (${err.error || err.status}): check OV_OAUTH_CLIENT_ID / OV_OAUTH_CLIENT_SECRET`);
++ return res.status(500).type('html').send(failurePage({
++ siteName: SITE_NAME,
++ title: 'Sign-in isn\'t working on OpenVibe.Reviews right now',
++ text: 'This is a problem on our side, not with your account, and it has been logged. Please try again a little later.',
++ retryHref: `/auth/login?next=${encodeURIComponent(next)}`,
++ backHref: next || '/',
++ }));
++ }
log.error(`[Reviews] code exchange failed: ${err.message}`);
return res.status(502).type('text/plain').send('Sign-in failed — could not reach OpenVibe.Network. Please try again.');
}
});
++ // GET /auth/health: does the Network accept this OAuth client? 503 only when it refuses it.
++ router.get('/health', async (_req, res) => {
++ const client = await sdkSso.checkClient();
++ res.set('Cache-Control', 'no-store');
++ return res.status(client === 'invalid' ? 503 : 200).json({ ok: client !== 'invalid', client });
++ });
++
router.post('/fedcm', express.json({ limit: '16kb' }), async (req, res) => {
const { token, nonce } = req.body || {};
if (typeof token !== 'string' || !token || !fedcmNonceMatches(token, nonce)) return res.status(400).json({ error: 'invalid_request', error_description: 'token and a matching nonce are required' });
try {
-- const data = await tokenGrant({ grant_type: 'urn:ietf:params:oauth:grant-type:jwt-bearer', assertion: token });
++ const data = await sdkSso.tokenGrant({ grant_type: 'urn:ietf:params:oauth:grant-type:jwt-bearer', assertion: token });
if (!data.access_token) throw Object.assign(new Error('no token'), { status: 401 });
setSession(res, data.access_token, data.refresh_token);
return res.json({ ok: true, user: data.user || null });
@@@ -188,7 +209,7 @@ function createSessionRoutes({ config, viewers, fetchImpl = globalThis.fetch, lo
const token = req.cookies && req.cookies[REFRESH_COOKIE];
if (!token) return res.status(401).json({ error: 'No refresh token' });
try {
-- const data = await tokenGrant({ grant_type: 'refresh_token', refresh_token: token });
++ const data = await sdkSso.tokenGrant({ grant_type: 'refresh_token', refresh_token: token });
setSession(res, data.access_token, data.refresh_token);
return res.json({ token: data.access_token });
} catch (err) {
@@@ -200,4 +221,4 @@ function createSessionRoutes({ config, viewers, fetchImpl = globalThis.fetch, lo
return router;
}
--module.exports = { createSessionRoutes, sanitizeNext, authorizeUrl, fedcmNonceMatches };
++module.exports = { createSessionRoutes, sanitizeNext, fedcmNonceMatches };
ddiff --git a/test/auth.test.js b/test/auth.test.js
nnew file mode 100644
iindex 0000000..be971ce
---- /dev/null
++++ b/test/auth.test.js
@@@ -0,0 +1,108 @@
++'use strict';
++/**
++ * The sign-in (openvibe-sdk/sso): the authorize URL carries PKCE S256, the flow cookies are the ones this site
++ * has always used (ov_oauth_state, ov_oauth_next, ov_oauth_silent and the PKCE verifier in ov_oauth_verifier),
++ * the state is compared with the flow cookie in constant time, next is sanitized at the login and at the
++ * callback, and GET /auth/health is 503 only when the Network refuses this site's OAuth client.
++ */
++const assert = require('assert');
++const crypto = require('crypto');
++const { boot, req, suite } = require('./helpers');
++
++const t = suite('auth');
++
++const cookies = (res) => res.headers.getSetCookie().map((c) => c.split(';')[0]);
++const cookieHeader = (res) => cookies(res).join('; ');
++const flow = (res, name) => res.headers.getSetCookie().find((c) => c.startsWith(name + '=')) || '';
++
++t('the sign-in starts at the Network with PKCE S256 and the flow cookies this site has always used', async () => {
++ const h = await boot();
++ try {
++ const res = await req(h, 'GET', '/auth/login?next=%2Fupdates');
++ assert.strictEqual(res.status, 302);
++ const u = new URL(res.headers.get('location'));
++ assert.deepStrictEqual([u.pathname, u.searchParams.get('client_id'), u.searchParams.get('redirect_uri'), u.searchParams.get('response_type')],
++ ['/oauth/authorize', 'reviews', 'http://reviews.test/auth/callback', 'code']);
++ assert.strictEqual(u.searchParams.get('code_challenge_method'), 'S256', 'PKCE S256 from openvibe-sdk/sso');
++ const challenge = u.searchParams.get('code_challenge');
++ assert.match(challenge, /^[A-Za-z0-9_-]{43}$/, 'the code challenge');
++ const state = u.searchParams.get('state');
++ assert.ok(state && state.length >= 16, 'the state');
++ assert.match(flow(res, 'ov_oauth_state'), new RegExp('^ov_oauth_state=' + state));
++ assert.match(flow(res, 'ov_oauth_verifier'), /Path=\/auth/i, 'the verifier stays in a flow cookie of its own');
++ // express encodes the cookie's value: the path arrives percent-encoded.
++ const nextCookie = flow(res, 'ov_oauth_next').split(';')[0].split('=')[1];
++ assert.strictEqual(decodeURIComponent(nextCookie), '/updates');
++ // The challenge is the SHA-256 of the verifier in the cookie: the Network checks exactly that.
++ const verifier = flow(res, 'ov_oauth_verifier').split(';')[0].split('=')[1];
++ assert.strictEqual(crypto.createHash('sha256').update(verifier).digest('base64url'), challenge);
++ } finally { await h.stop(); }
++});
++
++t('a callback whose state does not match the flow cookie is refused, and no session is set', async () => {
++ const h = await boot();
++ try {
++ const started = await req(h, 'GET', '/auth/login');
++ const res = await req(h, 'GET', '/auth/callback?code=whatever&state=not-the-state', { cookie: cookieHeader(started) });
++ assert.strictEqual(res.status, 400);
++ assert.ok(!res.headers.getSetCookie().some((c) => c.startsWith('ov_token=')), 'no session');
++ } finally { await h.stop(); }
++});
++
++t('a callback without the verifier cookie is refused too (the code could not be exchanged)', async () => {
++ const h = await boot();
++ try {
++ const started = await req(h, 'GET', '/auth/login');
++ const u = new URL(started.headers.get('location'));
++ const without = cookies(started).filter((c) => !c.startsWith('ov_oauth_verifier=')).join('; ');
++ const res = await req(h, 'GET', '/auth/callback?code=whatever&state=' + encodeURIComponent(u.searchParams.get('state')), { cookie: without });
++ assert.strictEqual(res.status, 400);
++ assert.ok(!res.headers.getSetCookie().some((c) => c.startsWith('ov_token=')), 'no session');
++ } finally { await h.stop(); }
++});
++
++t('a sign-in with a code the Network issued sets the session and returns to next', async () => {
++ const h = await boot();
++ try {
++ const started = await req(h, 'GET', '/auth/login?next=%2Fupdates');
++ const u = new URL(started.headers.get('location'));
++ const res = await req(h, 'GET', '/auth/callback?code=code_x&state=' + encodeURIComponent(u.searchParams.get('state')), { cookie: cookieHeader(started) });
++ assert.strictEqual(res.status, 302);
++ assert.strictEqual(res.headers.get('location'), '/updates');
++ const set = res.headers.getSetCookie();
++ assert.ok(set.some((c) => c.startsWith('ov_token=')), 'the session cookie, with the name this site has always used');
++ assert.ok(set.some((c) => c.startsWith('ov_refresh=')), 'the refresh cookie');
++ } finally { await h.stop(); }
++});
++
++t('next is sanitized: off-site never, neither at the login nor at the callback', async () => {
++ const h = await boot();
++ try {
++ const started = await req(h, 'GET', '/auth/login?next=https%3A%2F%2Fevil.com%2Fx');
++ const offSite = flow(started, 'ov_oauth_next').split(';')[0].split('=')[1];
++ assert.ok(!offSite, 'an off-site next is not kept');
++ const again = await req(h, 'GET', '/auth/login');
++ const u = new URL(again.headers.get('location'));
++ const res = await req(h, 'GET', '/auth/callback?code=code_x&state=' + encodeURIComponent(u.searchParams.get('state')),
++ { cookie: cookieHeader(again) + '; ov_oauth_next=//evil.com/x' });
++ assert.strictEqual(res.status, 302);
++ assert.strictEqual(res.headers.get('location'), '/', 'a tampered next goes home');
++ } finally { await h.stop(); }
++});
++
++t('GET /auth/health: 200 while the Network accepts this client, 503 when it refuses it', async () => {
++ const h = await boot();
++ try {
++ const res = await req(h, 'GET', '/auth/health');
++ assert.strictEqual(res.status, 200);
++ const json = JSON.parse(res.text);
++ assert.deepStrictEqual([json.ok, json.client], [true, 'ok']);
++ } finally { await h.stop(); }
++ const broken = await boot({ env: { OV_OAUTH_CLIENT_SECRET: 'not-the-secret' } });
++ try {
++ const res = await req(broken, 'GET', '/auth/health');
++ assert.strictEqual(res.status, 503);
++ const json = JSON.parse(res.text);
++ assert.deepStrictEqual([json.ok, json.client], [false, 'invalid']);
++ } finally { await broken.stop(); }
++});
ddiff --git a/test/helpers.js b/test/helpers.js
iindex fd818c8..021372b 100644
---- a/test/helpers.js
++++ b/test/helpers.js
@@@ -21,6 +21,8 @@ function subject(prefix = 'usr') { let s = ''; for (let i = 0; i < 26; i++) s +=
const EDITOR = subject();
const READER = subject();
const WEBHOOK_SECRET = 'test-webhook-secret-not-a-real-one';
++// The OAuth secret the stand-in Network knows: a client that presents another one is refused (invalid_client).
++const OAUTH_SECRET = 'test-service-secret-not-a-real-one';
function userToken({ subject: sub = READER, username = 'someone', role = 'user', aud = ['openvibe.network'], exp = 3600 } = {}) {
const now = Math.floor(Date.now() / 1000);
@@@ -151,7 +153,7 @@ async function boot({ env = {}, sources = fakeSources(), community = null, now,
const config = load({
NODE_ENV: 'test', PORT: '0', HOST: '127.0.0.1', BASE_URL: 'http://reviews.test',
OV_NETWORK_URL: ISSUER, OV_NETWORK_INTERNAL_URL: '', REVIEWS_GATE_MIN_WORDS: '20',
-- OV_OAUTH_CLIENT_SECRET: 'test-service-secret-not-a-real-one',
++ OV_OAUTH_CLIENT_SECRET: OAUTH_SECRET,
OV_SOURCES_INTERNAL_URL: 'http://sources.test', OV_COMMUNITY_INTERNAL_URL: 'http://community.test',
REVIEWS_EDITORS: EDITOR, REVIEWS_EVENTS_SECRET: WEBHOOK_SECRET,
...env,
@@@ -159,7 +161,13 @@ async function boot({ env = {}, sources = fakeSources(), community = null, now,
const fetchImpl = async (url, opts) => {
const u = new URL(url);
// The ingest chassis' Sources client mints a client-credentials token here (no network in tests).
-- if (u.pathname === '/oauth/token') return { status: 200, ok: true, json: async () => ({ access_token: 'stub-sources-token', token_type: 'Bearer', expires_in: 300, scope: 'sources.item.read sources.source.read' }) };
++ if (u.pathname === '/oauth/token') {
++ // The Network refuses a client whose secret is wrong (invalid_client): the sign-in's health check reads that.
++ let body = {};
++ try { body = JSON.parse((opts && opts.body) || '{}'); } catch { /* not JSON */ }
++ if (body.client_secret && body.client_secret !== OAUTH_SECRET) return { status: 401, ok: false, json: async () => ({ error: 'invalid_client' }) };
++ return { status: 200, ok: true, json: async () => ({ access_token: 'stub-sources-token', refresh_token: 'stub-refresh-token', token_type: 'Bearer', expires_in: 300, scope: 'sources.item.read sources.source.read' }) };
++ }
if (u.host === 'sources.test') return sources.fetch(url, opts);
if (u.host === 'community.test' && community) return community.fetch(url, opts);
throw new Error(`unexpected outbound fetch ${url}`);