From 5f475cefa6a3ca6820e6e81d65e98498f46ff4e4 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Mon, 31 Jul 2023 17:25:24 +0200 Subject: [PATCH] refactor: use process.env in node environments (#51110) --- api-server/src/common/models/user.js | 5 ++--- .../server/middlewares/constant-headers.js | 3 +-- api-server/src/server/middlewares/csp.js | 4 +--- .../middlewares/request-authorization.test.js | 7 ++++-- api-server/src/server/passport-providers.js | 7 +++--- api-server/src/server/utils/redirection.js | 15 ++++++++----- .../src/server/utils/redirection.test.js | 6 ++--- .../Introduction/components/block.tsx | 15 ++++++++----- curriculum/get-challenges.js | 22 +++++++++++++------ tools/scripts/build/download-trending.ts | 15 +++++++++---- utils/is-audited.js | 14 ++++++------ 11 files changed, 67 insertions(+), 46 deletions(-) diff --git a/api-server/src/common/models/user.js b/api-server/src/common/models/user.js index fdab43cd7dc..a60249a9503 100644 --- a/api-server/src/common/models/user.js +++ b/api-server/src/common/models/user.js @@ -16,7 +16,6 @@ import uuid from 'uuid/v4'; import { isEmail } from 'validator'; import { blocklistedUsernames } from '../../../../config/constants'; -import { apiLocation } from '../../../../config/env.json'; import { wrapHandledError } from '../../server/utils/create-handled-error.js'; import { @@ -202,7 +201,7 @@ export default function initializeUser(User) { exists => { if (exists) { throw wrapHandledError(new Error('user already exists'), { - redirectTo: `${apiLocation}/signin`, + redirectTo: `${process.env.API_LOCATION}/signin`, message: dedent` The ${user.email} email address is already associated with an account. Try signing in with it here instead. @@ -502,7 +501,7 @@ export default function initializeUser(User) { } const { id: loginToken, created: emailAuthLinkTTL } = token; const loginEmail = getEncodedEmail(newEmail ? newEmail : null); - const host = apiLocation; + const host = process.env.API_LOCATION; const mailOptions = { type: 'email', to: newEmail ? newEmail : this.email, diff --git a/api-server/src/server/middlewares/constant-headers.js b/api-server/src/server/middlewares/constant-headers.js index a26a2f71ebb..98ee5fa88de 100644 --- a/api-server/src/server/middlewares/constant-headers.js +++ b/api-server/src/server/middlewares/constant-headers.js @@ -1,5 +1,4 @@ import { allowedOrigins } from '../../../../config/cors-settings'; -import { homeLocation } from '../../../../config/env.json'; export default function constantHeaders() { return function (req, res, next) { @@ -10,7 +9,7 @@ export default function constantHeaders() { ) { res.header('Access-Control-Allow-Origin', req.headers.origin); } else { - res.header('Access-Control-Allow-Origin', homeLocation); + res.header('Access-Control-Allow-Origin', process.env.HOME_LOCATION); } res.header('Access-Control-Allow-Credentials', true); res.header( diff --git a/api-server/src/server/middlewares/csp.js b/api-server/src/server/middlewares/csp.js index d13f1ebdbae..4490368b970 100644 --- a/api-server/src/server/middlewares/csp.js +++ b/api-server/src/server/middlewares/csp.js @@ -1,11 +1,9 @@ import helmet from 'helmet'; -import { homeLocation } from '../../../../config/env.json'; - let trusted = [ "'self'", 'https://search.freecodecamp.org', - homeLocation, + process.env.HOME_LOCATION, 'https://' + process.env.AUTH0_DOMAIN ]; diff --git a/api-server/src/server/middlewares/request-authorization.test.js b/api-server/src/server/middlewares/request-authorization.test.js index 3ecfb3ad832..7b61b4f8212 100644 --- a/api-server/src/server/middlewares/request-authorization.test.js +++ b/api-server/src/server/middlewares/request-authorization.test.js @@ -1,11 +1,14 @@ +import path from 'path'; import jwt from 'jsonwebtoken'; +import { config } from 'dotenv'; -import { homeLocation } from '../../../../config/env.json'; import { mockReq as mockRequest, mockRes } from '../boot_tests/challenge.test'; import createRequestAuthorization, { isAllowedPath } from './request-authorization'; +config({ path: path.resolve(__dirname, '../../../../.env') }); + const validJWTSecret = 'this is a super secret string'; const invalidJWTSecret = 'This is not correct secret'; const now = new Date(Date.now()); @@ -27,7 +30,7 @@ const mockGetUserById = id => const mockReq = args => { const mock = mockRequest(args); - mock.header = () => homeLocation; + mock.header = () => process.env.HOME_LOCATION; return mock; }; diff --git a/api-server/src/server/passport-providers.js b/api-server/src/server/passport-providers.js index e6938162c41..d766c6f6d43 100644 --- a/api-server/src/server/passport-providers.js +++ b/api-server/src/server/passport-providers.js @@ -1,11 +1,10 @@ -import { homeLocation, apiLocation } from '../../../config/env.json'; import { auth0 } from '../../../config/secrets'; const { clientID, clientSecret, domain } = auth0; // These don't seem to be used, can they go? -const successRedirect = `${homeLocation}/learn`; -const failureRedirect = `${homeLocation}/signin`; +const successRedirect = `${process.env.HOME_LOCATION}/learn`; +const failureRedirect = `${process.env.HOME_LOCATION}/signin`; // TODO: can we remove passport-mock-strategy entirely in prod? That would let // us make passport-mock-strategy a dev dep, as it should be. @@ -33,7 +32,7 @@ const passportProviders = { clientSecret, domain, cookieDomain: process.env.COOKIE_DOMAIN || 'localhost', - callbackURL: `${apiLocation}/auth/auth0/callback`, + callbackURL: `${process.env.API_LOCATION}/auth/auth0/callback`, authPath: '/auth/auth0', callbackPath: '/auth/auth0/callback', useCustomCallback: true, diff --git a/api-server/src/server/utils/redirection.js b/api-server/src/server/utils/redirection.js index dd21ad48a61..4f075a068c5 100644 --- a/api-server/src/server/utils/redirection.js +++ b/api-server/src/server/utils/redirection.js @@ -1,11 +1,14 @@ const jwt = require('jsonwebtoken'); const { allowedOrigins } = require('../../../../config/cors-settings'); -// homeLocation is being used as a fallback here. If the one provided by the -// client is invalid we default to this. -const { homeLocation } = require('../../../../config/env.json'); +// process.env.HOME_LOCATION is being used as a fallback here. If the one +// provided by the client is invalid we default to this. const { availableLangs } = require('../../../../config/i18n'); -function getReturnTo(encryptedParams, secret, _homeLocation = homeLocation) { +function getReturnTo( + encryptedParams, + secret, + _homeLocation = process.env.HOME_LOCATION +) { let params; try { params = jwt.verify(encryptedParams, secret); @@ -25,7 +28,7 @@ function getReturnTo(encryptedParams, secret, _homeLocation = homeLocation) { function normalizeParams( { returnTo, origin, pathPrefix }, - _homeLocation = homeLocation + _homeLocation = process.env.HOME_LOCATION ) { // coerce to strings, just in case something weird and nefarious is happening returnTo = '' + returnTo; @@ -59,7 +62,7 @@ function getRedirectParams(req, _normalizeParams = normalizeParams) { const url = req.header('Referer'); // since we do not always redirect the user back to the page they were on // we need client locale and origin to construct the redirect url. - const returnUrl = new URL(url ? url : homeLocation); + const returnUrl = new URL(url ? url : process.env.HOME_LOCATION); const origin = returnUrl.origin; // if this is not one of the client languages, validation will convert // this to '' before it is used. diff --git a/api-server/src/server/utils/redirection.test.js b/api-server/src/server/utils/redirection.test.js index 8f3cfe89847..ce21db70806 100644 --- a/api-server/src/server/utils/redirection.test.js +++ b/api-server/src/server/utils/redirection.test.js @@ -67,7 +67,7 @@ describe('redirection', () => { expect(keys.length).toBe(3); expect(keys).toEqual(expect.arrayContaining(expectedKeys)); }); - it('should default to homeLocation', () => { + it('should default to process.env.HOME_LOCATION', () => { expect.assertions(1); expect(normalizeParams({}, defaultOrigin)).toEqual(defaultObject); }); @@ -92,9 +92,9 @@ describe('redirection', () => { ); }); // we *could*, in principle, grab the path and send them to - // homeLocation/path, but if the origin is wrong something unexpected is + // process.env.HOME_LOCATION/path, but if the origin is wrong something unexpected is // going on. In that case it's probably best to just send them to - // homeLocation/learn. + // process.env.HOME_LOCATION/learn. it('should return default parameters if the origin is unknown', () => { expect.assertions(1); const exampleOrigin = { diff --git a/client/src/templates/Introduction/components/block.tsx b/client/src/templates/Introduction/components/block.tsx index 88f7422b68f..0facfbec73d 100644 --- a/client/src/templates/Introduction/components/block.tsx +++ b/client/src/templates/Introduction/components/block.tsx @@ -30,7 +30,7 @@ import { import Challenges from './challenges'; import '../intro.css'; -const { curriculumLocale } = envData; +const { curriculumLocale, showUpcomingChanges, showNewCurriculum } = envData; const mapStateToProps = ( state: unknown, @@ -130,6 +130,11 @@ class Block extends Component { ); }); + const isAudited = isAuditedCert(curriculumLocale, superBlock, { + showNewCurriculum, + showUpcomingChanges + }); + const blockTitle = t(`intro:${superBlock}.blocks.${blockDashedName}.title`); // the real type of TFunction is the type below, because intro can be an array of strings // type RealTypeOFTFunction = TFunction & ((key: string) => string[]); @@ -160,7 +165,7 @@ class Block extends Component {

{blockTitle}

- {!isAuditedCert(curriculumLocale, superBlock) && ( + {isAudited && (
{

{blockTitle}

- {!isAuditedCert(curriculumLocale, superBlock) && ( + {isAudited && (
{
- {!isAuditedCert(curriculumLocale, superBlock) && ( + {isAudited && ( { - {!isAuditedCert(curriculumLocale, superBlock) && ( + {isAudited && ( `https://cdn.freecodecamp.org/universal/trending/${lang}.yaml`; @@ -44,7 +47,11 @@ const download = async (clientLocale: string) => { } }; -void download(clientLocale); +const locale = process.env.CLIENT_LOCALE; + +if (!locale) throw Error('CLIENT_LOCALE must be set to a valid locale'); + +void download(locale); // TODO: remove the need to fallback to english once we're confident it's // unnecessary (client/i18n/config.js will need all references to 'en' removing) -if (clientLocale !== 'english') void download('english'); +if (locale !== 'english') void download('english'); diff --git a/utils/is-audited.js b/utils/is-audited.js index e4406d5e818..504167afda8 100644 --- a/utils/is-audited.js +++ b/utils/is-audited.js @@ -1,16 +1,16 @@ const { getAuditedSuperBlocks } = require('../config/superblocks'); -const { - showNewCurriculum, - showUpcomingChanges -} = require('../config/env.json'); -function isAuditedCert(language, superblock) { +function isAuditedCert( + language, + superblock, + { showNewCurriculum, showUpcomingChanges } +) { if (!language || !superblock) throw Error('Both arguments must be provided for auditing'); const auditedSuperBlocks = getAuditedSuperBlocks({ - showNewCurriculum: showNewCurriculum.toString(), - showUpcomingChanges: showUpcomingChanges.toString(), + showNewCurriculum, + showUpcomingChanges, language }); return auditedSuperBlocks.includes(superblock);