gaspersic/freeCodeCamp
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

fix(csrf): remove all csrf bypass

Browse Source
This commit is contained in:
Oliver Eyton-Williams
2020-03-06 17:51:58 +01:00
committed by mrugesh
parent f183df0d88
commit 23b899f50f
20 changed files with 84 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
api-server/server/boot/certificate.js
View File

@ -37,7 +37,7 @@ export default function bootCertificate(app) {
api.put('/certificate/verify', ifNoUser401, ifNoSuperBlock404, verifyCert);
api.get('/certificate/showCert/:username/:cert', showCert);
app.use('/internal', api);
app.use(api);
}
const noNameMessage = dedent`

1
api-server/server/boot/challenge.js
View File

@ -61,7 +61,6 @@ export default async function bootChallenge(app, done) {
router.get('/map', redirectToLearn);
app.use(api);
app.use('/internal', api);
app.use(router);
done();
}

1
api-server/server/boot/donate.js
View File

@ -319,7 +319,6 @@ export default function donateBoot(app, done) {
api.post('/update-paypal', updatePaypal);
donateRouter.use('/donate', api);
app.use(donateRouter);
app.use('/internal', donateRouter);
connectToStripe().then(done);
}
}

5
api-server/server/boot/randomAPIs.js
View File

@ -9,7 +9,6 @@ const githubSecret = process.env.GITHUB_SECRET;
module.exports = function(app) {
const router = app.loopback.Router();
const api = app.loopback.Router();
const User = app.models.User;
router.get('/api/github', githubCalls);
@ -22,14 +21,12 @@ module.exports = function(app) {
);
router.get('/unsubscribed/:unsubscribeId', unsubscribedWithId);
router.get('/unsubscribed', unsubscribed);
api.get('/resubscribe/:unsubscribeId', resubscribe);
router.get('/resubscribe/:unsubscribeId', resubscribe);
router.get('/nonprofits', nonprofits);
router.get('/coding-bootcamp-cost-calculator', bootcampCalculator);
app.use(router);
app.use('/internal', api);
function theFastestWebPageOnTheInternet(req, res) {
res.render('resources/the-fastest-web-page-on-the-internet', {
title: 'This is the fastest web page on the internet'

1
api-server/server/boot/restApi.js
View File

@ -2,5 +2,4 @@ module.exports = function mountRestApi(app) {
const restApi = app.loopback.rest();
const restApiRoot = app.get('restApiRoot');
app.use(restApiRoot, restApi);
app.use(`/internal${restApiRoot}`, restApi);
};

1
api-server/server/boot/settings.js
View File

@ -48,7 +48,6 @@ export default function settingsController(app) {
api.put('/update-my-username', ifNoUser401, updateMyUsername);
api.put('/update-user-flag', ifNoUser401, updateUserFlag);
app.use('/internal', api);
app.use(api);
}

2
api-server/server/boot/user.js
View File

@ -31,7 +31,7 @@ function bootUser(app) {
api.post('/account/reset-progress', ifNoUser401, postResetProgress);
api.post('/user/report-user/', ifNoUser401, postReportUserProfile);
app.use('/internal', api);
app.use(api);
}
function createReadSessionUser(app) {

3
api-server/server/middlewares/csurf.js
View File

@ -6,10 +6,9 @@ export default function() {
domain: process.env.COOKIE_DOMAIN || 'localhost'
}
});
// Note: paypal webhook goes through /internal
return function csrf(req, res, next) {
const path = req.path.split('/')[1];
if (/(^api$|^unauthenticated$|^internal$|^p$)/.test(path)) {
if (/^donate\/update-paypal$/.test(path)) {
return next();
}
return protection(req, res, next);

26
api-server/server/middlewares/request-authorization.js
View File

@ -11,19 +11,23 @@ import { jwtSecret as _jwtSecret } from '../../../config/secrets';
import { wrapHandledError } from '../utils/create-handled-error';
// We need to tunnel through a proxy path set up within
// the gatsby app, at this time, that path is /internal
const apiProxyRE = /^\/internal\/|^\/external\//;
const newsShortLinksRE = /^\/internal\/n\/|^\/internal\/p\?/;
const loopbackAPIPathRE = /^\/internal\/api\//;
const showCertRe = /^\/internal\/certificate\/showCert\//;
const updatePaypalRe = /^\/internal\/donate\/update-paypal/;
const newsShortLinksRE = /^\/n\/|^\/p\//;
const showCertRE = /^\/certificate\/showCert\//;
const updatePaypalRE = /^\/donate\/update-paypal/;
// signin may not have a trailing slash
const signinRE = /^\/signin/;
const unsubscribeRE = /^\/u\/|^\/unsubscribe\/|^\/ue\//;
const unsubscribedRE = /^\/unsubscribed\//;
const resubscribeRE = /^\/resubscribe\//;
const _whiteListREs = [
newsShortLinksRE,
loopbackAPIPathRE,
showCertRe,
updatePaypalRe
showCertRE,
updatePaypalRE,
signinRE,
unsubscribeRE,
unsubscribedRE,
resubscribeRE
];
export function isWhiteListedPath(path, whiteListREs = _whiteListREs) {
@ -33,7 +37,7 @@ export function isWhiteListedPath(path, whiteListREs = _whiteListREs) {
export default ({ jwtSecret = _jwtSecret, getUserById = _getUserById } = {}) =>
function requestAuthorisation(req, res, next) {
const { path } = req;
if (apiProxyRE.test(path) && !isWhiteListedPath(path)) {
if (!isWhiteListedPath(path)) {
const { accessToken, error, jwt } = getAccessTokenFromRequest(
req,
jwtSecret

26
api-server/server/middlewares/request-authorization.test.js
View File

@ -64,7 +64,7 @@ describe('request-authorization', () => {
describe('cookies', () => {
it('throws when no access token is present', () => {
expect.assertions(2);
const req = mockReq({ path: '/internal/some-path/that-needs/auth' });
const req = mockReq({ path: '/some-path/that-needs/auth' });
const res = mockRes();
const next = sinon.spy();
expect(() => requestAuthorization(req, res, next)).toThrowError(
@ -77,7 +77,7 @@ describe('request-authorization', () => {
expect.assertions(2);
const invalidJWT = jwt.sign({ accessToken }, invalidJWTSecret);
const req = mockReq({
path: '/internal/some-path/that-needs/auth',
path: '/some-path/that-needs/auth',
// eslint-disable-next-line camelcase
cookie: { jwt_access_token: invalidJWT }
});
@ -97,7 +97,7 @@ describe('request-authorization', () => {
validJWTSecret
);
const req = mockReq({
path: '/internal/some-path/that-needs/auth',
path: '/some-path/that-needs/auth',
// eslint-disable-next-line camelcase
cookie: { jwt_access_token: invalidJWT }
});
@ -114,7 +114,7 @@ describe('request-authorization', () => {
expect.assertions(3);
const validJWT = jwt.sign({ accessToken }, validJWTSecret);
const req = mockReq({
path: '/internal/some-path/that-needs/auth',
path: '/some-path/that-needs/auth',
// eslint-disable-next-line camelcase
cookie: { jwt_access_token: validJWT }
});
@ -130,7 +130,7 @@ describe('request-authorization', () => {
it('adds the jwt to the headers', async done => {
const validJWT = jwt.sign({ accessToken }, validJWTSecret);
const req = mockReq({
path: '/internal/some-path/that-needs/auth',
path: '/some-path/that-needs/auth',
// eslint-disable-next-line camelcase
cookie: { jwt_access_token: validJWT }
});
@ -142,7 +142,8 @@ describe('request-authorization', () => {
});
it('calls next if request does not require authorization', async () => {
const req = mockReq({ path: '/unauthenticated/another/route' });
// currently /unsubscribe does not require authorization
const req = mockReq({ path: '/unsubscribe/another/route' });
const res = mockRes();
const next = sinon.spy();
await requestAuthorization(req, res, next);
@ -153,7 +154,7 @@ describe('request-authorization', () => {
describe('Auth header', () => {
it('throws when no access token is present', () => {
expect.assertions(2);
const req = mockReq({ path: '/internal/some-path/that-needs/auth' });
const req = mockReq({ path: '/some-path/that-needs/auth' });
const res = mockRes();
const next = sinon.spy();
expect(() => requestAuthorization(req, res, next)).toThrowError(
@ -166,7 +167,7 @@ describe('request-authorization', () => {
expect.assertions(2);
const invalidJWT = jwt.sign({ accessToken }, invalidJWTSecret);
const req = mockReq({
path: '/internal/some-path/that-needs/auth',
path: '/some-path/that-needs/auth',
headers: { 'X-fcc-access-token': invalidJWT }
});
const res = mockRes();
@ -185,7 +186,7 @@ describe('request-authorization', () => {
validJWTSecret
);
const req = mockReq({
path: '/internal/some-path/that-needs/auth',
path: '/some-path/that-needs/auth',
headers: { 'X-fcc-access-token': invalidJWT }
});
const res = mockRes();
@ -201,7 +202,7 @@ describe('request-authorization', () => {
expect.assertions(3);
const validJWT = jwt.sign({ accessToken }, validJWTSecret);
const req = mockReq({
path: '/internal/some-path/that-needs/auth',
path: '/some-path/that-needs/auth',
headers: { 'X-fcc-access-token': validJWT }
});
const res = mockRes();
@ -216,7 +217,7 @@ describe('request-authorization', () => {
it('adds the jwt to the headers', async done => {
const validJWT = jwt.sign({ accessToken }, validJWTSecret);
const req = mockReq({
path: '/internal/some-path/that-needs/auth',
path: '/some-path/that-needs/auth',
// eslint-disable-next-line camelcase
cookie: { jwt_access_token: validJWT }
});
@ -228,7 +229,8 @@ describe('request-authorization', () => {
});
it('calls next if request does not require authorization', async () => {
const req = mockReq({ path: '/unauthenticated/another/route' });
// currently /unsubscribe does not require authorization
const req = mockReq({ path: '/unsubscribe/another/route' });
const res = mockRes();
const next = sinon.spy();
await requestAuthorization(req, res, next);

2
api-server/server/views/emails/user-request-sign-in.ejs
View File

@ -1,6 +1,6 @@
Here's your sign in link. It will instantly sign you into freeCodeCamp.org - no password necessary:
<%= host %>/internal/passwordless-auth/?email=<%= loginEmail %>&token=<%= loginToken %>
<%= host %>/passwordless-auth/?email=<%= loginEmail %>&token=<%= loginToken %>
Note: this sign in link will expire after 15 minutes. If you need a new sign in link, go to https://www.freecodecamp.org/signin

2
api-server/server/views/emails/user-request-sign-up.ejs
View File

@ -4,7 +4,7 @@ We have created a new account for you.
Here's your sign in link. It will instantly sign you into freeCodeCamp.org - no password necessary:
<%= host %>/internal/passwordless-auth/?email=<%= loginEmail %>&token=<%= loginToken %>
<%= host %>/passwordless-auth/?email=<%= loginEmail %>&token=<%= loginToken %>
Note: this sign in link will expire after 15 minutes. If you need a new sign in link, go to https://www.freecodecamp.org/signin

33
client/package-lock.json generated
View File

@ -8141,6 +8141,16 @@
"resolved": "https://registry.npmjs.org/crypto-random-string/-/crypto-random-string-1.0.0.tgz",
"integrity": "sha1-ojD2T1aDEOFJgAmUB5DsmVRbyn4="
},
"csrf": {
"version": "3.1.0",
"resolved": "https://registry.npmjs.org/csrf/-/csrf-3.1.0.tgz",
"integrity": "sha512-uTqEnCvWRk042asU6JtapDTcJeeailFy4ydOQS28bj1hcLnYRiqi8SsD2jS412AY1I/4qdOwWZun774iqywf9w==",
"requires": {
"rndm": "1.2.0",
"tsscmp": "1.0.6",
"uid-safe": "2.1.5"
}
},
"css": {
"version": "2.2.4",
"resolved": "https://registry.npmjs.org/css/-/css-2.2.4.tgz",
@ -22220,6 +22230,11 @@
"ret": "~0.1.10"
}
},
"random-bytes": {
"version": "1.0.0",
"resolved": "https://registry.npmjs.org/random-bytes/-/random-bytes-1.0.0.tgz",
"integrity": "sha1-T2ih3Arli9P7lYSMMDJNt11kNgs="
},
"randombytes": {
"version": "2.1.0",
"resolved": "https://registry.npmjs.org/randombytes/-/randombytes-2.1.0.tgz",
@ -23989,6 +24004,11 @@
"inherits": "^2.0.1"
}
},
"rndm": {
"version": "1.2.0",
"resolved": "https://registry.npmjs.org/rndm/-/rndm-1.2.0.tgz",
"integrity": "sha1-8z/pz7Urv9UgqhgyO8ZdsRCht2w="
},
"rst-selector-parser": {
"version": "2.2.3",
"resolved": "https://registry.npmjs.org/rst-selector-parser/-/rst-selector-parser-2.2.3.tgz",
@ -26173,6 +26193,11 @@
"resolved": "https://registry.npmjs.org/tslib/-/tslib-1.10.0.tgz",
"integrity": "sha512-qOebF53frne81cf0S9B41ByenJ3/IuH8yJKngAX35CmiZySA0khhkovshKK+jGCaMnVomla7gVlIcc3EvKPbTQ=="
},
"tsscmp": {
"version": "1.0.6",
"resolved": "https://registry.npmjs.org/tsscmp/-/tsscmp-1.0.6.tgz",
"integrity": "sha512-LxhtAkPDTkVCMQjt2h6eBVY28KCjikZqZfMcC15YBeNjkgUpdCfBu5HoiOTDu86v6smE8yOjyEktJ8hlbANHQA=="
},
"tsutils": {
"version": "3.17.1",
"resolved": "https://registry.npmjs.org/tsutils/-/tsutils-3.17.1.tgz",
@ -26286,6 +26311,14 @@
"resolved": "https://registry.npmjs.org/ua-parser-js/-/ua-parser-js-0.7.17.tgz",
"integrity": "sha512-uRdSdu1oA1rncCQL7sCj8vSyZkgtL7faaw9Tc9rZ3mGgraQ7+Pdx7w5mnOSF3gw9ZNG6oc+KXfkon3bKuROm0g=="
},
"uid-safe": {
"version": "2.1.5",
"resolved": "https://registry.npmjs.org/uid-safe/-/uid-safe-2.1.5.tgz",
"integrity": "sha512-KPHm4VL5dDXKz01UuEd88Df+KzynaohSL9fBh096KWAxSKZQDI2uBrVqtvRM4rwrIrRRKsdLNML/lnaaVSRioA==",
"requires": {
"random-bytes": "~1.0.0"
}
},
"unc-path-regex": {
"version": "0.1.2",
"resolved": "https://registry.npmjs.org/unc-path-regex/-/unc-path-regex-0.1.2.tgz",

1
client/package.json
View File

@ -22,6 +22,7 @@
"bezier-easing": "^2.1.0",
"browser-cookies": "^1.2.0",
"chai": "^4.2.0",
"csrf": "^3.1.0",
"date-fns": "^1.30.1",
"entities": "^1.1.2",
"enzyme": "^3.10.0",

2
client/src/client-only-routes/ShowUnsubscribed.js
View File

@ -31,7 +31,7 @@ function ShowUnsubscribed({ unsubscribeId }) {
block={true}
bsSize='lg'
bsStyle='primary'
href={`${apiLocation}/internal/resubscribe/${unsubscribeId}`}
href={`${apiLocation}/resubscribe/${unsubscribeId}`}
>
You can click here to resubscribe
</Button>

1
client/src/redux/index.js
View File

@ -115,6 +115,7 @@ export const preventProgressDonationRequests = createAction(
export const onlineStatusChange = createAction(types.onlineStatusChange);
// TODO: re-evaluate this since /internal is no longer used.
// `hardGoTo` is used to hit the API server directly
// without going through /internal
// used for things like /signin and /signout

14
client/src/utils/ajax.js
View File

@ -1,14 +1,18 @@
import { apiLocation } from '../../config/env.json';
import { _csrf } from '../redux/cookieValues';
import axios from 'axios';
import Tokens from 'csrf';
const base = apiLocation + '/internal';
const baseUnauthenticated = apiLocation + '/unauthenticated';
const base = apiLocation;
const tokens = new Tokens();
axios.defaults.withCredentials = true;
export function postUnauthenticated(path, body) {
return axios.post(`${baseUnauthenticated}${path}`, body);
// _csrf is passed to the client as a cookie. Tokens are sent back to the server
// via headers:
if (_csrf) {
axios.defaults.headers.post['CSRF-Token'] = tokens.create(_csrf);
axios.defaults.headers.put['CSRF-Token'] = tokens.create(_csrf);
}
function get(path) {

4
tools/scripts/build/__snapshots__/create-redirects.test.js.snap
View File

@ -10,10 +10,6 @@ exports[`createRedirects matches the snapshot 1`] = `
https://freecodecamp-dev.netlify.com/* https://www.freecodecamp.dev/:splat 301!
https://freecodecamp-org.netlify.com/* https://www.freecodecamp.org/:splat 301!
#api redirect
/internal/* https://api.example.com/internal/:splat 200!
# pages
/about https://news.example.com/about 200
/academic-honesty https://news.example.com/academic-honesty 200

4
tools/scripts/build/create-redirects.js
View File

@ -31,10 +31,6 @@ const template = `
https://freecodecamp-dev.netlify.com/* https://www.freecodecamp.dev/:splat 301!
https://freecodecamp-org.netlify.com/* https://www.freecodecamp.org/:splat 301!
#api redirect
/internal/* #{{API}}/internal/:splat 200!
# pages
/about #{{NEWS}}/about 200
/academic-honesty #{{NEWS}}/academic-honesty 200

5
tools/scripts/build/create-redirects.test.js
View File

@ -18,7 +18,7 @@ describe('createRedirects', () => {
});
it('replaces instances of `#{{...}}` with the locations provided', () => {
expect.assertions(5);
expect.assertions(4);
const apiPlaceholderRE = /#\{\{API\}\}/;
const newsPlaceholderRE = /#\{\{NEWS\}\}/;
@ -33,8 +33,7 @@ describe('createRedirects', () => {
expect(hasNewsPlaceholder).toBe(false);
expect(hasForumPlaceholder).toBe(false);
const { api, forumProxy } = testLocations;
expect(redirects.includes(`${api}/internal/:splat`)).toBe(true);
const { forumProxy } = testLocations;
expect(redirects.includes(`${forumProxy}`)).toBe(true);
});