Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: sign redirect with secret from env var #5415

Merged
merged 1 commit into from Jan 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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,
charliegroll marked this conversation as resolved.
Show resolved Hide resolved
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