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.

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

Anonymise logs #3310

Closed
wants to merge 1 commit into from
Closed

Anonymise logs #3310

wants to merge 1 commit into from

Conversation

lukeyouell
Copy link

Due to GDPR it would be really handy if we were able to anonymise the log prefixes.

This PR allows for log anonymisation to be enabled from the general config file.

@brandonkelly
Copy link
Member

@lukeyouell you mainly just don't want the IP in there right?

@lukeyouell
Copy link
Author

lukeyouell commented Sep 18, 2018

That's correct, but the simplest solution would be to remove the user & session ID's too.

I can alter the code to keep these if you'd prefer?

@brandonkelly
Copy link
Member

Thinking we will just override getMessagePrefix() and strip out the IP. Doubt anyone is relying on that.

@lukeyouell
Copy link
Author

We've used the IP address for debugging webhooks in the past so I do think there's a place for it in development environments.

@brandonkelly
Copy link
Member

Took it in a slightly different direction, because I’d rather avoid creating a new config setting for this. So now craft\log\FileTarget has a new $includeUserIp property, which is false by default.

If you really need to show IP addresses for debugging purposes or whatever, you can enable it from config/app.php:

return [
    'components' => [
        'log' => function() {
            $config = craft\helpers\App::logConfig();
            if ($config) {
                $config['targets'][0]['includeUserIp'] = true;
                return Craft::createObject($config);
            }
            return null;
        },
    ],
];

@lukeyouell
Copy link
Author

Sounds good to me! Cheers Brandon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants