From 7fa6b9ca031813e4b2604581b1474df6f2e129f0 Mon Sep 17 00:00:00 2001 From: Tobias Bocanegra Date: Tue, 14 Jun 2022 14:20:38 +0200 Subject: [PATCH] fix: handle xfh properly and protect forms and json pipeline --- src/forms-pipe.js | 5 +++ src/json-pipe.js | 2 ++ src/steps/authenticate.js | 11 ++++-- src/utils/auth.js | 14 +++++++- test/forms-pipe.test.js | 9 +++++ test/json-pipe.test.js | 7 ++++ test/steps/authenticate.test.js | 20 +++++++++++ test/utils/auth.test.js | 60 +++++++++++++++++++++++++++++++-- 8 files changed, 122 insertions(+), 6 deletions(-) diff --git a/src/forms-pipe.js b/src/forms-pipe.js index 467f4eb8..f1001aa9 100644 --- a/src/forms-pipe.js +++ b/src/forms-pipe.js @@ -13,6 +13,7 @@ import { cleanupHeaderValue } from '@adobe/helix-shared-utils'; import { PipelineResponse } from './PipelineResponse.js'; import fetchConfigAll from './steps/fetch-config-all.js'; import setCustomResponseHeaders from './steps/set-custom-response-headers.js'; +import { authenticate } from './steps/authenticate.js'; function error(log, msg, status, response) { log.error(msg); @@ -96,6 +97,10 @@ export async function formsPipe(state, request) { }, }); await fetchConfigAll(state, request, response); + await authenticate(state, request, response); + if (response.error) { + return response; + } await setCustomResponseHeaders(state, request, response); const { diff --git a/src/json-pipe.js b/src/json-pipe.js index 7347666e..c81e1037 100644 --- a/src/json-pipe.js +++ b/src/json-pipe.js @@ -14,6 +14,7 @@ import setCustomResponseHeaders from './steps/set-custom-response-headers.js'; import { PipelineResponse } from './PipelineResponse.js'; import jsonFilter from './utils/json-filter.js'; import { extractLastModified, updateLastModified } from './utils/last-modified.js'; +import { authenticate } from './steps/authenticate.js'; /** * Runs the default pipeline and returns the response. @@ -80,6 +81,7 @@ export async function jsonPipe(state, req) { // Load config-all and set response headers await fetchConfigAll(state, req, response); + await authenticate(state, req, response); await setCustomResponseHeaders(state, req, response); return response; diff --git a/src/steps/authenticate.js b/src/steps/authenticate.js index 63aeb37b..d50489c8 100644 --- a/src/steps/authenticate.js +++ b/src/steps/authenticate.js @@ -37,7 +37,7 @@ export function isAllowed(email = '', allows = []) { */ export async function authenticate(state, req, res) { // get auth info - const authInfo = await getAuthInfo(state, req, res); + const authInfo = await getAuthInfo(state, req); // check if `.auth` route to validate and exchange token if (state.info.path === '/.auth') { @@ -52,12 +52,17 @@ export async function authenticate(state, req, res) { // if not authenticated, redirect to login screen if (!authInfo.authenticated) { + // send 401 for plain requests + if (state.info.selector || state.type !== 'html') { + state.log.warn('[auth] unauthorized. redirect to login only for extension less html.'); + res.status = 401; + res.error = 'unauthorized.'; + return; + } authInfo.redirectToLogin(state, req, res); return; } - // console.log(authInfo.profile); - // check profile is allowed const { allow } = state.config.access; const allows = Array.isArray(allow) ? allow : [allow]; diff --git a/src/utils/auth.js b/src/utils/auth.js index e2651698..0bb0358a 100644 --- a/src/utils/auth.js +++ b/src/utils/auth.js @@ -150,7 +150,19 @@ export class AuthInfo { // determine the location of 'this' document based on the xfh header. so that logins to // .page stay on .page. etc. but fallback to the config.host if non set - const host = req.headers.get('x-forwarded-host') || state.config.host; + let host = req.headers.get('x-forwarded-host'); + if (host) { + host = host.split(',')[0].trim(); + } + if (!host) { + host = state.config.host; + } + if (!host) { + log.error('[auth] unable to create login redirect: no xfh or config.host.'); + res.status = 401; + res.error = 'no host information.'; + return; + } const url = new URL(idp.discovery.authorization_endpoint); diff --git a/test/forms-pipe.test.js b/test/forms-pipe.test.js index adeea1f9..63119c3f 100644 --- a/test/forms-pipe.test.js +++ b/test/forms-pipe.test.js @@ -262,6 +262,15 @@ describe('Form POST Requests', () => { assert.strictEqual(resp.status, 400); }); + it('reject unauthorized.', async () => { + const req = new PipelineRequest('https://helix-pipeline.com/', defaultRequest); + const state = new PipelineState(defaultState()); + state.config.access = { allow: '*@adobe.com' }; + state.s3Loader = new StaticS3Loader(); + const resp = await formsPipe(state, req); + assert.strictEqual(resp.status, 401); + }); + describe('extractBodyData', () => { const validBody = { data: { diff --git a/test/json-pipe.test.js b/test/json-pipe.test.js index a0529d2f..2650df72 100644 --- a/test/json-pipe.test.js +++ b/test/json-pipe.test.js @@ -315,6 +315,13 @@ describe('JSON Pipe Test', () => { assert.strictEqual(resp.headers.get('x-error'), 'multisheet data invalid. missing ":names" property.'); }); + it('rejects unauthorized', async () => { + const state = createDefaultState(); + state.config.access = { allow: '*@adobe.com' }; + const resp = await jsonPipe(state, new PipelineRequest('https://json-filter.com/?limit=10')); + assert.strictEqual(resp.status, 401); + }); + it('creates correct filter with no offset', async () => { const state = createDefaultState(); const resp = await jsonPipe(state, new PipelineRequest('https://json-filter.com/?limit=10')); diff --git a/test/steps/authenticate.test.js b/test/steps/authenticate.test.js index c35d6831..25d4e862 100644 --- a/test/steps/authenticate.test.js +++ b/test/steps/authenticate.test.js @@ -52,6 +52,26 @@ describe('Authenticate Test', () => { assert.strictEqual(res.status, 302); }); + it('send 401 for unauthenticated .plain requests', async () => { + const { authenticate: authProxy } = await esmock('../../src/steps/authenticate.js', { + '../../src/utils/auth.js': { + getAuthInfo: () => ({ + redirectToLogin(state, req, res) { + res.status = 302; + }, + }), + }, + }); + const state = new PipelineState({ path: '/nav.plain.html' }); + state.config.access = { + allow: '*@adobe.com', + }; + const req = new PipelineRequest('https://localhost'); + const res = new PipelineResponse(); + await authProxy(state, req, res); + assert.strictEqual(res.status, 401); + }); + it('.auth fetches the token', async () => { const { authenticate: authProxy } = await esmock('../../src/steps/authenticate.js', { '../../src/utils/auth.js': { diff --git a/test/utils/auth.test.js b/test/utils/auth.test.js index eb3c0253..ce048104 100644 --- a/test/utils/auth.test.js +++ b/test/utils/auth.test.js @@ -13,7 +13,7 @@ /* eslint-env mocha */ import assert from 'assert'; import { - generateKeyPair, exportJWK, SignJWT, UnsecuredJWT, + generateKeyPair, exportJWK, SignJWT, UnsecuredJWT, decodeJwt, } from 'jose'; import { getAuthInfo, @@ -310,6 +310,7 @@ describe('AuthInfo tests', () => { .withIdp(idpFakeTestIDP); const state = new PipelineState({}); + state.config.host = 'www.hlx.live'; const req = new PipelineRequest('https://localhost'); const res = new PipelineResponse(); await authInfo.redirectToLogin(state, req, res); @@ -327,10 +328,65 @@ describe('AuthInfo tests', () => { redirect_uri: 'https://login.hlx.page/.auth', response_type: 'code', scope: 'openid profile email', - state: 'eyJhbGciOiJub25lIn0.eyJyZXF1ZXN0UGF0aCI6Ii8ifQ.', + state: 'eyJhbGciOiJub25lIn0.eyJyZXF1ZXN0UGF0aCI6Ii8iLCJyZXF1ZXN0SG9zdCI6Ind3dy5obHgubGl2ZSJ9.', }); }); + it('redirects to the login page (xfh)', async () => { + const authInfo = AuthInfo + .Default() + .withIdp(idpFakeTestIDP); + + const state = new PipelineState({}); + const req = new PipelineRequest('https://localhost', { + headers: { + 'x-forwarded-host': 'www.hlx.live', + }, + }); + const res = new PipelineResponse(); + await authInfo.redirectToLogin(state, req, res); + assert.strictEqual(res.status, 302); + const reqState = new URL(res.headers.get('location')).searchParams.get('state'); + assert.deepStrictEqual(decodeJwt(reqState), { + requestHost: 'www.hlx.live', + requestPath: '/', + }); + }); + + it('redirects to the login page (xfh - multi)', async () => { + const authInfo = AuthInfo + .Default() + .withIdp(idpFakeTestIDP); + + const state = new PipelineState({ path: '/en/blog' }); + const req = new PipelineRequest('https://localhost/', { + headers: { + 'x-forwarded-host': 'bla.live, foo.page', + }, + }); + const res = new PipelineResponse(); + await authInfo.redirectToLogin(state, req, res); + assert.strictEqual(res.status, 302); + const reqState = new URL(res.headers.get('location')).searchParams.get('state'); + assert.deepStrictEqual(decodeJwt(reqState), { + requestHost: 'bla.live', + requestPath: '/en/blog', + }); + }); + + it('redirects fails if not host', async () => { + const authInfo = AuthInfo + .Default() + .withIdp(idpFakeTestIDP); + + const state = new PipelineState({}); + const req = new PipelineRequest('https://localhost/'); + const res = new PipelineResponse(); + await authInfo.redirectToLogin(state, req, res); + assert.strictEqual(res.status, 401); + assert.strictEqual(res.error, 'no host information.'); + }); + it('redirects to the login page needs client id', async () => { const authInfo = AuthInfo .Default()