Skip to content

Commit

Permalink
fix: sign redirect with secret from env var (#5415)
Browse files Browse the repository at this point in the history
  • Loading branch information
eduardoboucas committed Jan 20, 2023
1 parent 027efcf commit 99305de
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 15 deletions.
36 changes: 24 additions & 12 deletions src/utils/proxy.mjs
Expand Up @@ -29,7 +29,7 @@ import {
import { fileExistsAsync, isFileAsync } from '../lib/fs.mjs'
import renderErrorTemplate from '../lib/render-error-template.mjs'

import { NETLIFYDEVLOG, NETLIFYDEVWARN } from './command-helpers.mjs'
import { NETLIFYDEVLOG, NETLIFYDEVWARN, log, chalk } from './command-helpers.mjs'
import createStreamPromise from './create-stream-promise.mjs'
import { headersForPath, parseHeaders } from './headers.mjs'
import { createRewriter, onChanges } from './rules-proxy.mjs'
Expand Down Expand Up @@ -144,7 +144,7 @@ const alternativePathsFor = function (url) {
return paths
}

const serveRedirect = async function ({ match, options, proxy, req, res, siteInfo }) {
const serveRedirect = async function ({ env, match, options, proxy, req, res, siteInfo }) {
if (!match) return proxy.web(req, res, options)

options = options || req.proxyOptions || {}
Expand All @@ -157,12 +157,21 @@ const serveRedirect = async function ({ match, options, proxy, req, res, siteInf
}

if (match.signingSecret) {
req.headers['x-nf-sign'] = signRedirect({
deployContext: 'dev',
secret: match.signingSecret,
siteID: siteInfo.id,
siteURL: siteInfo.url,
})
const signingSecretVar = env[match.signingSecret]

if (signingSecretVar) {
req.headers['x-nf-sign'] = signRedirect({
deployContext: 'dev',
secret: signingSecretVar.value,
siteID: siteInfo.id,
siteURL: siteInfo.url,
})
} else {
log(
NETLIFYDEVWARN,
`Could not sign redirect because environment variable ${chalk.yellow(match.signingSecret)} is not set`,
)
}
}

if (isFunction(options.functionsPort, req.url)) {
Expand Down Expand Up @@ -316,7 +325,7 @@ const reqToURL = function (req, pathname) {

const MILLISEC_TO_SEC = 1e3

const initializeProxy = async function ({ configPath, distDir, host, port, projectDir, siteInfo }) {
const initializeProxy = async function ({ configPath, distDir, env, host, port, projectDir, siteInfo }) {
const proxy = httpProxy.createProxyServer({
selfHandleResponse: true,
target: {
Expand Down Expand Up @@ -387,13 +396,14 @@ const initializeProxy = async function ({ configPath, distDir, host, port, proje
match: req.proxyOptions.match,
options: req.proxyOptions,
siteInfo,
env,
})
}
}

if (req.proxyOptions.staticFile && isRedirect({ status: proxyRes.statusCode }) && proxyRes.headers.location) {
req.url = proxyRes.headers.location
return serveRedirect({ req, res, proxy: handlers, match: null, options: req.proxyOptions, siteInfo })
return serveRedirect({ req, res, proxy: handlers, match: null, options: req.proxyOptions, siteInfo, env })
}

const responseData = []
Expand Down Expand Up @@ -490,7 +500,7 @@ const initializeProxy = async function ({ configPath, distDir, host, port, proje
}

const onRequest = async (
{ addonsUrls, edgeFunctionsProxy, functionsServer, proxy, rewriter, settings, siteInfo },
{ addonsUrls, edgeFunctionsProxy, env, functionsServer, proxy, rewriter, settings, siteInfo },
req,
res,
) => {
Expand Down Expand Up @@ -530,7 +540,7 @@ const onRequest = async (
// We don't want to generate an ETag for 3xx redirects.
req[shouldGenerateETag] = ({ statusCode }) => statusCode < 300 || statusCode >= 400

return serveRedirect({ req, res, proxy, match, options, siteInfo })
return serveRedirect({ req, res, proxy, match, options, siteInfo, env })
}

// The request will be served by the framework server, which means we want to
Expand Down Expand Up @@ -586,6 +596,7 @@ export const startProxy = async function ({
state,
})
const proxy = await initializeProxy({
env,
host: settings.frameworkHost,
port: settings.frameworkPort,
distDir: settings.dist,
Expand All @@ -611,6 +622,7 @@ export const startProxy = async function ({
functionsServer,
edgeFunctionsProxy,
siteInfo,
env,
})
const primaryServer = settings.https
? https.createServer({ cert: settings.https.cert, key: settings.https.key }, onRequestWithOptions)
Expand Down
9 changes: 6 additions & 3 deletions tests/integration/0.command.dev.test.cjs
Expand Up @@ -203,7 +203,7 @@ test('should rewrite requests to an external server', async (t) => {

test('should sign external redirects with the `x-nf-sign` header when a `signed` value is set', async (t) => {
await withSiteBuilder('site-redirects-file-to-external', async (builder) => {
const mockSigningKey = 'iamverysecret'
const mockSigningSecret = 'iamverysecret'
const externalServer = startExternalServer()
const { port } = externalServer.address()
const siteInfo = {
Expand All @@ -224,7 +224,10 @@ test('should sign external redirects with the `x-nf-sign` header when a `signed`
await builder
.withNetlifyToml({
config: {
redirects: [{ from: '/sign/*', to: `http://localhost:${port}/:splat`, signed: mockSigningKey, status: 200 }],
build: { environment: { VAR_WITH_SIGNING_SECRET: mockSigningSecret } },
redirects: [
{ from: '/sign/*', to: `http://localhost:${port}/:splat`, signed: 'VAR_WITH_SIGNING_SECRET', status: 200 },
],
},
})
.buildAsync()
Expand Down Expand Up @@ -254,7 +257,7 @@ test('should sign external redirects with the `x-nf-sign` header when a `signed`

;[getResponse, postResponse].forEach((response) => {
const signature = response.headers['x-nf-sign']
const payload = jwt.verify(signature, mockSigningKey)
const payload = jwt.verify(signature, mockSigningSecret)

t.is(payload.deploy_context, 'dev')
t.is(payload.netlify_id, siteInfo.id)
Expand Down

1 comment on commit 99305de

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📊 Benchmark results

  • Package size: 267 MB

Please sign in to comment.