Provide an optional logger callback: logger({ logType, ...logData }}

[timtong1982]

Any plan to add custom logger?

or I can send a PR

[psibean]

Can you provide a bit more detail on how you expect this to look?

We don't want to add any logging dependency here, I think the only way to make this generic would be to:

1. Use an event emitter to emit events at certain points throughout the middleware e.g. onGenerateNewToken, onReuseExistingToken, onInvalidToken; or
2. Allow callbacks to be passed in to be called at the same points, this pollutes the config a lot more than using an event emitter.

An event emitter could be backwards compatible and could be a good thing to have.

[timtong1982]

thanks for the rely!

let's say I want to know what is the exactly reason caused validateRequest failure

proposal:

1. Add a logger func input as optional parameter in DoubleCsrfConfigOptions, like type Logger = (eventType, message) => void
2. While assert failed or other informational logs which needed, and the logger is available, we can call the logger and let the caller handling the log
   e.g.

const validateRequest: CsrfRequestValidator = (req) => {
    // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
    const csrfCookie = getCsrfCookieFromRequest(req);
    if (typeof csrfCookie !== "string") {
logger && logger('error', 'csrfCookie type is incorrecgt');
return false;
      }

    // cookie has the form {token}|{hash}
    const [csrfToken, csrfTokenHash] = csrfCookie.split("|");

    // csrf token from the request
    const csrfTokenFromRequest = getTokenFromRequest(req) as string;

    const getSecretResult = getSecret(req);
    const possibleSecrets = Array.isArray(getSecretResult)
      ? getSecretResult
      : [getSecretResult];

    return (
      csrfToken === csrfTokenFromRequest &&
      validateTokenAndHashPair(
        csrfTokenFromRequest,
        csrfTokenHash,
        possibleSecrets,
      )
    );
  };

[psibean]

Yep, I think events are going to be a better option. I don't want csrf-csrf to assume the implementation of a logger.

So what I'm proposing instead would look something like this:

    if (typeof csrfCookie !== "string") {
        eventEmitter.emit('csrfCookieInvalid', req, csrfCookie);
        return false;
    }

And it would allow you to do something like this:

eventEmitter.on('csrfCookieInvalid', (req, csrfCookie) => {
    logger('error', 'csrfCookie type is incorrect');
});

Where eventEmitter is now exported from csrf-csrf

To achieve the same thing, the benefit here is, the events emitted can be used to do anything at particular steps of the CSRF middleware, not just log. It also keeps any logging users want to do out of the middleware itself and in complete control of the user.

This event approach also sets things up to make the default export a class instance which extends the Node EventEmitter class in the next major version. But this could go into v3 as a minor update by exporting an eventEmitter instance.

[timtong1982]

sounds good! thanks!

[chr15m]

It would be great if there was an option for this. It could even be as simple as console.error of the reason why csrf-csrf is rejecting the token. Seeing this in the logs:

ForbiddenError: invalid csrf token

And not knowing where to start can be a frustrating experience. In general the more that is logged the better when things fail.

[psibean]

I attempted an event emitter based approach and found it didn't work too well.

I think what I'll do now is provide a logger callback option, the logger will be called throughout csrf-csrf, something like this:

// wrap the passed in callback 
const csrfLogger = (logData) => {
    if (logger) logger({ logData });
}

Then just do something like:

csrfLogger({ logType:  'GENERATED_TOKEN', req, csrfToken });

And just call it at the different points throughout the code, pass in relevant data, and have a logType that helps you determine when/where the logger was called to understand what is happening. The log types will be documented. Being an optional config means this won't be a breaking change, so I should be able to add the logger option to both v3 and v4.

The logger should be used with caution though, as you generally shouldn't log any sensitive information, it's mostly just provided to make debugging easier.

https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Logging_Cheat_Sheet.md

[psibean]

I haven't yet introduced this feature as getting the version 4 release out was top priority, and that focus was on the breaking changes.

This logging feature can be implemented in a backwards compatible way, I'll look into it some time soon (hopefully, I make no promises).

[psibean]

I've raised #107 as a proof of concept to addressing this issue.

I'm still a bit cautious on merging this in at this point in time, I may need to wait and see if there is a higher demand for this.

In the mean time if anyone has any feedback regarding the implementation/approach in the PR, feel free to comment here.