hapijs / crumb

CSRF crumb generation and validation for hapi
Other
171 stars 51 forks source link

Setting `plugins.crumb: false` on a route does not disable crumb validation #165

Open fjeldstad opened 3 weeks ago

fjeldstad commented 3 weeks ago

Runtime

node.js

Runtime version

20

Module version

9.0.1

Last module version without issue

No response

Used with

hapi

Any other relevant information

From the documentation:

Additionally, some configuration can be passed on a per-route basis. Disable Crumb for a particular route by passing false instead of a configuration object.

This test case verifies the expected behaviour with regards to setting route.options.plugins.crumb: false:

it('does not validate crumb when route.options.plugins.crumb is false', async () => {
    const server = Hapi.server();
    server.route({
        method: 'POST',
        path: '/1',
        options: {
            plugins: {
                crumb: false
            }
        },
        handler: (request, h) => 'test'
    });
    const plugins = [
        {
            plugin: Crumb,
        }
    ];
    await server.register(plugins);
    const headers = {
        'X-API-Token': 'test'
    };
    const res = await server.inject({
        method: 'POST',
        url: '/1',
        headers
    });
    const header = res.headers['set-cookie'];
    expect(res.statusCode).to.equal(200);
    expect(header).to.not.exist();
});

What are you trying to achieve or the steps to reproduce?

I want to disable crumb validation/generation for a specific route, without using the skip option (to keep concerns separated). I therefore set route.options.plugins.crumb: false as suggested by the documentation.

What was the result you got?

The crumb validation runs and a new cookie value is returned.

What result did you expect?

The crumb validation should not run, no cookie should be set.

fjeldstad commented 3 weeks ago

Created a PR, but I'm not sure if it solves it in the most appropriate way. I thought an early return would be safest + it allows for reducing nesting in the plugin, but I might well have missed something. https://github.com/hapijs/crumb/pull/166

fjeldstad commented 3 weeks ago

The best workaround I've come up with is to set a custom skip function that patches the bug (it will result in an early return):

skip: (request, h) => request?.route?.settings?.plugins?.crumb === false,