Skip to content

Commit

Permalink
Add ability to specify CSRF token in header
Browse files Browse the repository at this point in the history
  • Loading branch information
@matthew-white
matthew-white committed Jul 13, 2023
1 parent 52bb6d2 commit 65fe31a
Show file tree
Hide file tree
Showing 3 changed files with 198 additions and 59 deletions.
16 changes: 12 additions & 4 deletions lib/http/preprocessors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,22 @@ const authHandler = ({ Sessions, Users, Auth, bcrypt }, context) => {
// if authentication failed anyway, just do nothing.
if ((cxt == null) || !cxt.auth.session.isDefined()) return;

// The CSRF token may be specified in a header or in the request body. We
// used to require the token to be in the request body, but that isn't
// compatible with all request bodies (for example, a file). There are
// times when Central specifies the token in a header, and other times
// when it specifies it in the request body: it depends on the use case.
const csrf = cxt.headers['x-csrf-token'] ?? cxt.body.__csrf;

// if csrf missing or mismatch; fail outright.
const csrf = cxt.body.__csrf;
if (isBlank(csrf) || (cxt.auth.session.get().csrf !== decodeURIComponent(csrf)))
return reject(Problem.user.authenticationFailed());

// delete the token off the body so it doesn't mess with downstream
// payload expectations.
return cxt.with({ body: without([ '__csrf' ], cxt.body) });
return cxt.headers['x-csrf-token'] != null
? cxt
// delete the token off the body so it doesn't mess with downstream
// payload expectations.
: cxt.with({ body: without([ '__csrf' ], cxt.body) });
});
}
};
Expand Down
74 changes: 57 additions & 17 deletions test/integration/api/sessions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,19 @@ describe('api: /sessions', () => {

// this isn't exactly the right place for this but i just want to check the
// whole stack in addition to the unit tests.
describe('cookie CSRF auth', () => {
describe('CSRF protection for cookie auth used for non-GET requests', () => {
it('should reject with a 403 if session token is invalid', testService(async (service) => {
const { body: session } = await service.post('/v1/sessions')
.send({ email: 'alice@getodk.org', password: 'alice' })
.expect(200);
await service.post('/v1/projects')
.send({ name: 'my project' })
.set('X-Forwarded-Proto', 'https')
.set('Cookie', '__Host-session=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa')
.set('X-Csrf-Token', session.csrf)
.expect(403);
}));

it('should reject if the CSRF token is missing', testService((service) =>
service.post('/v1/sessions')
.send({ email: 'alice@getodk.org', password: 'alice' })
Expand All @@ -282,25 +294,53 @@ describe('api: /sessions', () => {
.set('Cookie', '__Host-session=' + body.token)
.expect(401))));

it('should reject if the CSRF token is wrong', testService((service) =>
service.post('/v1/sessions')
.send({ email: 'alice@getodk.org', password: 'alice' })
.expect(200)
.then(({ body }) => service.post('/v1/projects')
.send({ name: 'my project', __csrf: 'nope' })
describe('CSRF token in the request body', () => {
it('should reject if the CSRF token is wrong', testService((service) =>
service.post('/v1/sessions')
.send({ email: 'alice@getodk.org', password: 'alice' })
.expect(200)
.then(({ body }) => service.post('/v1/projects')
.send({ name: 'my project', __csrf: 'nope' })
.set('X-Forwarded-Proto', 'https')
.set('Cookie', '__Host-session=' + body.token)
.expect(401))));

it('should succeed if the CSRF token is correct', testService((service) =>
service.post('/v1/sessions')
.send({ email: 'alice@getodk.org', password: 'alice' })
.expect(200)
.then(({ body }) => service.post('/v1/projects')
.send({ name: 'my project', __csrf: body.csrf })
.set('X-Forwarded-Proto', 'https')
.set('Cookie', '__Host-session=' + body.token)
.expect(200))));
});

describe('CSRF token in a header', () => {
it('should reject if the token is incorrect', testService(async (service) => {
const { body: session } = await service.post('/v1/sessions')
.send({ email: 'alice@getodk.org', password: 'alice' })
.expect(200);
await service.post('/v1/projects')
.send({ name: 'my project' })
.set('X-Forwarded-Proto', 'https')
.set('Cookie', '__Host-session=' + body.token)
.expect(401))));
.set('Cookie', `__Host-session=${session.token}`)
.set('X-Csrf-Token', 'nope')
.expect(401);
}));

it('should succeed if the CSRF token is correct', testService((service) =>
service.post('/v1/sessions')
.send({ email: 'alice@getodk.org', password: 'alice' })
.expect(200)
.then(({ body }) => service.post('/v1/projects')
.send({ name: 'my project', __csrf: body.csrf })
it('should succeed if the token is correct', testService(async (service) => {
const { body: session } = await service.post('/v1/sessions')
.send({ email: 'alice@getodk.org', password: 'alice' })
.expect(200);
await service.post('/v1/projects')
.send({ name: 'my project' })
.set('X-Forwarded-Proto', 'https')
.set('Cookie', '__Host-session=' + body.token)
.expect(200))));
.set('Cookie', `__Host-session=${session.token}`)
.set('X-Csrf-Token', session.csrf)
.expect(200);
}));
});
});
});

167 changes: 129 additions & 38 deletions test/unit/http/preprocessors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ describe('preprocessors', () => {
context.auth.session.should.eql(Option.of(new Session({ test: 'session' })));
}));

describe('CSRF protection', () => {
describe('CSRF protection for non-GET requests', () => {
const mockSessionsWithCsrf = (expectedToken, csrf) => ({
getByBearerToken: (token) => Promise.resolve((token === expectedToken)
? Option.of(new Session({ csrf }))
Expand All @@ -298,18 +298,6 @@ describe('preprocessors', () => {
)
)).should.be.rejectedWith(Problem, { problemCode: 401.2 }));

it('should reject cookie auth with incorrect CSRF token for non-GET requests', () =>
Promise.resolve(authHandler(
{ Auth, Sessions: mockSessionsWithCsrf('alohomora', 'secretcsrf') },
new Context(
createRequest({ method: 'POST', headers: {
'X-Forwarded-Proto': 'https',
Cookie: '__Host-session=alohomora'
}, body: { __csrf: 'notsecretcsrf' } }),
{ fieldKey: Option.none() }
)
)).should.be.rejectedWith(Problem, { problemCode: 401.2 }));

it('should do nothing on cookie auth with incorrect session token for non-GET requests', () =>
Promise.resolve(authHandler(
{ Auth, Sessions: mockSessionsWithCsrf('alohomora', 'secretcsrf') },
Expand All @@ -325,17 +313,134 @@ describe('preprocessors', () => {
should.not.exist(context);
}));

it('should accept cookie auth with correct CSRF token for non-GET requests', () =>
Promise.resolve(authHandler(
{ Auth, Sessions: mockSessionsWithCsrf('alohomora', 'secretcsrf') },
new Context(
createRequest({ method: 'POST', headers: {
'X-Forwarded-Proto': 'https',
Cookie: '__Host-session=alohomora'
}, body: { __csrf: 'secretcsrf' } }),
{ fieldKey: Option.none() }
)
)).should.be.fulfilled());
describe('CSRF token in request body', () => {
it('should reject cookie auth with incorrect CSRF token for non-GET requests', () =>
Promise.resolve(authHandler(
{ Auth, Sessions: mockSessionsWithCsrf('alohomora', 'secretcsrf') },
new Context(
createRequest({ method: 'POST', headers: {
'X-Forwarded-Proto': 'https',
Cookie: '__Host-session=alohomora'
}, body: { __csrf: 'notsecretcsrf' } }),
{ fieldKey: Option.none() }
)
)).should.be.rejectedWith(Problem, { problemCode: 401.2 }));

it('should accept cookie auth with correct CSRF token for non-GET requests', () =>
Promise.resolve(authHandler(
{ Auth, Sessions: mockSessionsWithCsrf('alohomora', 'secretcsrf') },
new Context(
createRequest({ method: 'POST', headers: {
'X-Forwarded-Proto': 'https',
Cookie: '__Host-session=alohomora'
}, body: { __csrf: 'secretcsrf' } }),
{ fieldKey: Option.none() }
)
)).should.be.fulfilled());

it('should remove CSRF token from data payload on success', () =>
Promise.resolve(authHandler(
{ Auth, Sessions: mockSessionsWithCsrf('alohomora', 'secretcsrf') },
new Context(
createRequest({ method: 'POST', headers: {
'X-Forwarded-Proto': 'https',
Cookie: '__Host-session=alohomora'
}, body: { __csrf: 'secretcsrf', other: 'data' } }),
{ fieldKey: Option.none() }
)
)).then((context) => {
context.body.should.eql({ other: 'data' });
}));
});

describe('CSRF token in header', () => {
it('should reject if the token is incorrect', () =>
Promise.resolve(authHandler(
{ Auth, Sessions: mockSessionsWithCsrf('alohomora', 'secretcsrf') },
new Context(
createRequest({
method: 'POST',
headers: {
'X-Forwarded-Proto': 'https',
Cookie: '__Host-session=alohomora',
'X-Csrf-Token': 'notsecretcsrf'
}
}),
{ fieldKey: Option.none() }
)
)).should.be.rejectedWith(Problem, { problemCode: 401.2 }));

it('should accept if the token is correct', () =>
Promise.resolve(authHandler(
{ Auth, Sessions: mockSessionsWithCsrf('alohomora', 'secretcsrf') },
new Context(
createRequest({
method: 'POST',
headers: {
'X-Forwarded-Proto': 'https',
Cookie: '__Host-session=alohomora',
'X-Csrf-Token': 'secretcsrf'
}
}),
{ fieldKey: Option.none() }
)
)).should.be.fulfilled());
});

describe('precedence of CSRF token in header over request body', () => {
it('should accept if token in header is correct even if token in request body is not', () =>
Promise.resolve(authHandler(
{ Auth, Sessions: mockSessionsWithCsrf('alohomora', 'secretcsrf') },
new Context(
createRequest({
method: 'POST',
headers: {
'X-Forwarded-Proto': 'https',
Cookie: '__Host-session=alohomora',
'X-Csrf-Token': 'secretcsrf'
},
body: { __csrf: 'notsecretcsrf' }
}),
{ fieldKey: Option.none() }
)
)).should.be.fulfilled());

it('should reject if token in header is incorrect even if token in request body is correct', () =>
Promise.resolve(authHandler(
{ Auth, Sessions: mockSessionsWithCsrf('alohomora', 'secretcsrf') },
new Context(
createRequest({
method: 'POST',
headers: {
'X-Forwarded-Proto': 'https',
Cookie: '__Host-session=alohomora',
'X-Csrf-Token': 'notsecretcsrf'
},
body: { __csrf: 'secretcsrf' }
}),
{ fieldKey: Option.none() }
)
)).should.be.rejectedWith(Problem, { problemCode: 401.2 }));

it('should not remove token from request body if header was specified', async () => {
const context = await authHandler(
{ Auth, Sessions: mockSessionsWithCsrf('alohomora', 'secretcsrf') },
new Context(
createRequest({
method: 'POST',
headers: {
'X-Forwarded-Proto': 'https',
Cookie: '__Host-session=alohomora',
'X-Csrf-Token': 'secretcsrf'
},
body: { __csrf: 'secretcsrf', other: 'data' }
}),
{ fieldKey: Option.none() }
)
);
context.body.should.eql({ __csrf: 'secretcsrf', other: 'data' });
});
});

it('should url-decode the CSRF token', () =>
Promise.resolve(authHandler(
Expand All @@ -348,20 +453,6 @@ describe('preprocessors', () => {
{ fieldKey: Option.none() }
)
)).should.be.fulfilled());

it('should remove CSRF token from data payload on success', () =>
Promise.resolve(authHandler(
{ Auth, Sessions: mockSessionsWithCsrf('alohomora', 'secretcsrf') },
new Context(
createRequest({ method: 'POST', headers: {
'X-Forwarded-Proto': 'https',
Cookie: '__Host-session=alohomora'
}, body: { __csrf: 'secretcsrf', other: 'data' } }),
{ fieldKey: Option.none() }
)
)).then((context) => {
context.body.should.eql({ other: 'data' });
}));
});
});

Expand Down

0 comments on commit 65fe31a

Please sign in to comment.