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.

Sign up for GitHub

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

Jump to bottom

feat: add rotateWhenInvalid option for CSRF token #98

Merged
merged 2 commits into from
Jul 3, 2024
Merged

feat: add rotateWhenInvalid option for CSRF token #98

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f315dd1
feat: add rotateWhenInvalid option for CSRF token
hongzzz Jul 2, 2024
3fe56e0
chore: add document for rotateWhenInvalid option
hongzzz Jul 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ exports.security = {
headerName: 'x-csrf-token', // request csrf token's name in header
bodyName: '_csrf', // request csrf token's name in body
queryName: '_csrf', // request csrf token's name in query
rotateWhenInvalid: false, // rotate csrf secret when csrf token invalid
refererWhiteList: [], // referer white list
supportedRequests: [ // supported URL path and method, the package will match URL path regex patterns one by one until path matched. We recommend you set {path: /^\//, methods:['POST','PATCH','DELETE','PUT','CONNECT']} as the last rule in the list, which is also the default config.
{path: /^\//, methods:['POST','PATCH','DELETE','PUT','CONNECT']}
Expand Down
1 change: 1 addition & 0 deletions README.zh-CN.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ exports.security = {
headerName: 'x-csrf-token', // csrf token 在 header 中的名称
bodyName: '_csrf', // csrf token 在 body 中的名称
queryName: '_csrf', // csrf token 在 query 中的名称
rotateWhenInvalid: false, // csrf invalid 时刷新 token
fengmk2 marked this conversation as resolved.
Show resolved Hide resolved
refererWhiteList: [], // referer 白名单
supportedRequests: [ // 支持的 url path pattern 和方法，根据配置名单由上至下匹配 url path 正则，建议在自定义时配置 {path: /^\//, methods:['POST','PATCH','DELETE','PUT','CONNECT']} 为兜底规则
{path: /^\//, methods:['POST','PATCH','DELETE','PUT','CONNECT']},
Expand Down
4 changes: 4 additions & 0 deletions app/extend/context.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@ module.exports = {
if (token !== this[CSRF_SECRET] && !tokens.verify(this[CSRF_SECRET], token)) {
debug('verify secret and token error');
this[LOG_CSRF_NOTICE]('invalid csrf token');
const { rotateWhenInvalid } = this.app.config.security.csrf;
if (rotateWhenInvalid) {
this.rotateCsrfSecret();
}
return 'invalid csrf token';
}
},
Expand Down
1 change: 1 addition & 0 deletions config/config.default.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ module.exports = () => {
headerName: 'x-csrf-token',
bodyName: '_csrf',
queryName: '_csrf',
rotateWhenInvalid: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Add documentation for the new rotateWhenInvalid option.

Ensure that the new rotateWhenInvalid option is documented within the comments for the csrf configuration.

   * @property {Object} csrf - whether defend csrf attack
   * @property {Object} xframe - whether enable X-Frame-Options response header, default SAMEORIGIN
   * @property {Object} hsts - whether enable Strict-Transport-Security response header, default is one year
   * @property {Object} methodnoallow - whether enable Http Method filter
   * @property {Object} noopen - whether enable IE automaticlly download open
   * @property {Object} nosniff -  whether enable IE8 automaticlly dedect mime
   * @property {Object} xssProtection -  whether enable IE8 XSS Filter, default is open
   * @property {Object} csp - content security policy config
   * @property {Object} referrerPolicy - referrer policy config
   * @property {Object} dta - auto avoid directory traversal attack
   * @property {Array} domainWhiteList - domain white list
   * @property {Array} protocolWhiteList - protocal white list
   * @property {Boolean} rotateWhenInvalid - rotate CSRF secret when token is invalid
   */
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
rotateWhenInvalid: false,
* @property {Object} csrf - whether defend csrf attack
* @property {Object} xframe - whether enable X-Frame-Options response header, default SAMEORIGIN
* @property {Object} hsts - whether enable Strict-Transport-Security response header, default is one year
* @property {Object} methodnoallow - whether enable Http Method filter
* @property {Object} noopen - whether enable IE automaticlly download open
* @property {Object} nosniff - whether enable IE8 automaticlly dedect mime
* @property {Object} xssProtection - whether enable IE8 XSS Filter, default is open
* @property {Object} csp - content security policy config
* @property {Object} referrerPolicy - referrer policy config
* @property {Object} dta - auto avoid directory traversal attack
* @property {Array} domainWhiteList - domain white list
* @property {Array} protocolWhiteList - protocal white list
* @property {Boolean} rotateWhenInvalid - rotate CSRF secret when token is invalid

supportedRequests: [
{ path: /^\//, methods: [ 'POST', 'PATCH', 'DELETE', 'PUT', 'CONNECT' ] },
],
Expand Down
11 changes: 11 additions & 0 deletions test/csrf.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,17 @@ describe('test/csrf.test.js', () => {
});
});

it('token should be rotated when enable rotateWhenInvalid', async () => {
mm(app.config.security.csrf, 'rotateWhenInvalid', true);
await app.httpRequest()
.post('/update')
.set('x-csrf-token', '2')
.set('cookie', 'csrfToken=1')
.send({ title: 'invalid token' })
.expect(403)
.expect(res => assert(!!res.header['set-cookie']));
});
Comment on lines +286 to +295
Copy link
Contributor

Choose a reason for hiding this comment

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

Add comments for clarity.

The test function is well-structured and correctly tests the new functionality. Adding comments will improve readability and maintainability.

+  // Test to check if the CSRF token is rotated when `rotateWhenInvalid` is enabled
  it('token should be rotated when enable rotateWhenInvalid', async () => {
+    // Mock the `rotateWhenInvalid` configuration to true
    mm(app.config.security.csrf, 'rotateWhenInvalid', true);
+    // Perform an HTTP request with an invalid CSRF token
    await app.httpRequest()
      .post('/update')
      .set('x-csrf-token', '2')
      .set('cookie', 'csrfToken=1')
      .send({ title: 'invalid token' })
      .expect(403)
+      // Assert that the CSRF token is rotated by checking the `set-cookie` header
      .expect(res => assert(!!res.header['set-cookie']));
  });
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('token should be rotated when enable rotateWhenInvalid', async () => {
mm(app.config.security.csrf, 'rotateWhenInvalid', true);
await app.httpRequest()
.post('/update')
.set('x-csrf-token', '2')
.set('cookie', 'csrfToken=1')
.send({ title: 'invalid token' })
.expect(403)
.expect(res => assert(!!res.header['set-cookie']));
});
// Test to check if the CSRF token is rotated when `rotateWhenInvalid` is enabled
it('token should be rotated when enable rotateWhenInvalid', async () => {
// Mock the `rotateWhenInvalid` configuration to true
mm(app.config.security.csrf, 'rotateWhenInvalid', true);
// Perform an HTTP request with an invalid CSRF token
await app.httpRequest()
.post('/update')
.set('x-csrf-token', '2')
.set('cookie', 'csrfToken=1')
.send({ title: 'invalid token' })
.expect(403)
// Assert that the CSRF token is rotated by checking the `set-cookie` header
.expect(res => assert(!!res.header['set-cookie']));
});


it('should show deprecate message if ignoreJSON = true', async () => {
const app = mm.app({ baseDir: 'apps/csrf-ignorejson' });
await app.ready();
Expand Down
Loading