Support for Non-401 Error Codes from the validateFunc

[Eldritch-Oliver]

Support plan

• is this issue currently blocking your project? (yes/no): No
• is this issue affecting a production system? (yes/no): No

Context

• node version: v14.17.0
• module version: 11.0.2
• environment (e.g. node, browser, native): Node
• used with (e.g. hapi application, another framework, standalone, ...): Hapi application (Hapi version = 20.2.1)
• any other relevant information:

What problem are you trying to solve?

I have a system that involves multiple different "regions" of authorization, I want a cookie to only be valid for one of these "regions", and I have added validation checks into the validateFunc. I would love be able to respond to the client with a 403 Forbidden when the cookie provided is for a different "region" than that which it is trying to access.

Example:
I have "regions" 1 and 2, and an authorization cookie is used for region 1, but the user is making a request to GET /region/2 I would like to be able to throw boom.forbidden(), and it set the response code to 403 instead of the plugin only throwing 401 to the user.

Do you have a new or modified API suggestion to solve the problem?

I think a solution following a similar vein as to how @hapi/basic does it where if the validateFunc throws an error(/Boom error) it replaces the default boom.unauthorized()

From the @hapi/basic API documentation for the validate function:

• Throwing an error from this function will replace default Boom.unauthorized error

[devinivy]

In the code and tests, the module is fairly explicit about what it's trying to do when it sees a non-unauthorized error:

cookie/test/index.js

Lines 374 to 414 in fa728d7

	it('errors in validation function', async () => {
	const server = Hapi.server();
	await server.register(require('../'));
	server.auth.strategy('default', 'cookie', {
	cookie: {
	password: 'password-should-be-32-characters',
	clearInvalid: true,
	ttl: 60 * 1000,
	name: 'special'
	},
	validateFunc: function (request, session) {
	throw new Error('boom');
	}
	});
	server.auth.default('default');
	Helpers.loginWithResourceEndpoint(server);
	const res = await server.inject('/login/steve');
	expect(res.result).to.equal('steve');
	const header = res.headers['set-cookie'];
	expect(header.length).to.equal(1);
	expect(header[0]).to.contain('Max-Age=60');
	const cookie = header[0].match(internals.cookieRx);
	let error;
	server.ext('onPreResponse', (request, h) => {
	error = request.response.data;
	return h.continue;
	});
	const res2 = await server.inject({ method: 'GET', url: '/resource', headers: { cookie: 'special=' + cookie[1] } });
	expect(res2.statusCode).to.equal(401);
	expect(error).to.be.an.error('boom');
	});

That means that we'd need to treat this as a breaking change, most likely.

Since the original error is preserved on the error's data property, you do have an option to get this behavior using a request extension. Something like this should do the trick https://runkit.com/devinivy/627987b9369f5200089451e0:

server.ext({
    type: 'onPreResponse',
    method: (request, h) => {
        const error = request.response;
        if (Boom.isBoom(error) && error.output.statusCode === 401 && error.data instanceof Error) {
            // Preserve original error from Boom.unauthorized()
            return error.data;
        }
        return h.continue;
    }
});

[Eldritch-Oliver]

Okay, that makes sense, I can definitely see this being suited as a breaking change since it does change a substantial amount of the existing behaviour.

Thanks for the snippet! It seems like a suitable stand-in for how I'm wanting the erroring to behave.

I think this could be a nice thing to include in a future major-version release, but I am also satisfied with this resolution if this isn't something that is desired within the API.

[sholladay]

I think the behavior discussed in this thread is the cause of a more general problem that I have seen.

In our validate function, we run a database query to see if the user still exists, as is recommended in the docs. Most of the time, it works fine. However, if there's a connection failure to the database, the database client throws an error, and errors in validate seem to log the user out no matter what.

I don't think it makes sense to log the user out just because the backend can't contact the database. Probably a 500 error should be returned, with the user still logged in so they can try again.