User credential changes are cached by OPcache

[neildaniels]

Describe the bug
User credentials (email, language, name, role) are store in an .php file in the accounts directory. Because they're stored as a PHP file, they can fall under the control of OPcache.

If opcache.validate_timestamps is set to true, this won't cause an issue.

However, in my Production setup, I disable opcache.validate_timestamps for performance reasons. The result of this is: changes to user credentials are silently "ignored" until I reset the server process.

To Reproduce
Steps to reproduce the behavior:

1. Have opcache.validate_timestamps = 0 in your php.ini file.
2. Go to an existing user's page in the Panel
3. Change this existing user's email address (note: changing it via the Panel or directly on disk are somewhat distinct here; see below)
4. Refresh the Panel page

Expected behavior
The existing user's page should always reflect what is on disk.

In the situation of using the Panel, I would expect that writeCredentials() to follow with an opcache_invalidate() call. Thus, something like:

    protected function writeCredentials(array $credentials): bool
    {
        $path = $this->root() . '/index.php';
        if ($result = Data::write(path, $credentials)) {
            opcache_invalidate($path, true);
        }
        return $result;
    }

However, the above solution will be ineffective if you make a change directly on disk—or, in my situation, you have multiple Docker instances sharing the same accounts directory. (In this situation, only the server that processed the API request to change the credentials would have it's cached; there other instances would never know of the invalidation.)

You could move the opcache_invalidate to readCredentials() method, and call it every time before the require. This seems a bit silly, but should work. (I suspect manually reading the PHP file (i.e. not using require/include) would work, but then you're left with the task of parsing a PHP file.)

I think a more tenable (albeit breaking) solution would be to move away from using a PHP file to store credentials. You can keep an empty file to prevent directory sniffing, but move the actual contents to a separate yaml/json file.

Kirby Version
3.3.5

Additional context

[lukasbestle]

To be honest, I think that disabling opcache.validate_timestamps and manually updating the user files is quite an edge-case. The main advantage of using PHP files is the performance for many users – loading PHP files is just quite a bit faster than parsing YAML or JSON. So giving up this advantage just for this edge-case is likely not going to be worth it.

But besides that: You are definitely right that we should invalidate the cache after writing to the file. Fixed in #2635. :)

[lukasbestle]

@neildaniels Based on what you wrote in #2635: Does this still require a fix for your setup?

[neildaniels]

My setup has multiple docker instances, so technically it’s still broken.

I assume most people don’t use Kirby in this way, so I understand that it’s a relatively rare scenario.

I do think it’s a bit odd that this is seemingly the only place where a PHP file is used for dynamic-ish content. (Languages do too, but I disable Panel customization of those.)

Neither locks nor sessions use PHP files.

However, I understand that accounts are somewhat unique in that for login to work, you need to run through each one to find a matching email address, so parsing speed is more important.

I don’t have any compelling suggestions, aside from maybe a feature request to force text files (and be less performant).

[lukasbestle]

@bastianallgeier can better explain the thoughts behind choosing the PHP format. Maybe I missed something and it would actually be viable to switch to a more static config format.

[bastianallgeier]

One reason was performance and the second reason was a last resort in terms of security. Should the server configuration be so bad or our htaccess rules be removed or ignored and a visitor would be able to guess the user id and open /site/accounts/xyz in the browser the result would just be an empty page. With a json or text file the server would just send the content of it to the browser.

[lukasbestle]

Right, I remember now! So to be honest, I think we should close this issue then.

[bastianallgeier]

I agree. Sorry @neildaniels!

[neildaniels]

Finally realized there's a decent way to handle this:

Add the accounts directory to an OPCache Blacklist file. https://www.php.net/manual/en/opcache.configuration.php#ini.opcache.blacklist-filename

php.ini file:

opcache.blacklist_filename = /path/to/blocklist.txt

blocklist.txt:

/path/to/kirby/website/storage/accounts/*

Both paths will vary depending on your installation setup.