Fix caching routes by users with an active session

[danxuliu]

Fixes #56789, which is a regression introduced in #52793

When a user has an active session only the apps that are enabled for the user are initially loaded*. In order to cache the routes the routes for all apps are loaded, but routes defined in routes.php are taken into account only if the app was already loaded. Therefore, when the routes were cached in a request by a user with an active session only the routes for apps enabled for that user were cached, and those routes were used by any other user, independently of which apps they had access to. To solve that now all the enabled apps are explicitly loaded before caching the routes.

Note that this did not affect routes defined using annotations on the controller files; in that case the loaded routes do not depend on the previously loaded apps, as it explicitly checks all the enabled apps.

*As soon as the session is initialized, which happens when loading base.php, the legacy OC_APP::getEnabledApps will return only the apps enabled for the user. That method is used by AppManager::loadApps, so once the session is initialized any load of (several) apps will be restricted to those enabled for the user (explicitly loading a single app still works as expected). Therefore, when $appManager->loadApps() is called from the OCS handler or from the index.php handler (through handleRequest in base.php) only the apps for the user are loaded.

Steps to reproduce

• Use a development server and ensure that no other requests than the ones below are handled
• Enable APCu cache (add 'memcache.local' => '\\OC\\Memcache\\APCu', to config.php) if not enabled already
• Enable the weather_status app only for a specific group (for simplicity admin is used here)

occ app:enable --groups admin weather_status`

• Adjust the variables below as needed for your setup and run the following Bash code:

export ADMIN_NAME=admin
export ADMIN_PASSWORD=admin
export USER_NAME=user0
export USER_PASSWORD=user0
export SERVER_URL=http://127.0.0.1:8000

function extractRequestToken() {
	echo $(echo $1 | grep --perl-regexp --only-matching 'data-requesttoken="\K([^"]*)')
}

function extractAuthenticationToken() {
	echo $(echo $1 | grep --perl-regexp --only-matching '"token":"\K([^"]*)')
}

# Open login page to get the request token
loginPage=$(curl --silent --cookie-jar "/tmp/cookie-jar-$USER_NAME" "$SERVER_URL/index.php/login")

requestToken=$(extractRequestToken "$loginPage")

# Perform actual login
loginPost=$(curl --silent --location --cookie "/tmp/cookie-jar-$USER_NAME" --cookie-jar "/tmp/cookie-jar-$USER_NAME" --header "Origin: $SERVER_URL" --data-urlencode "user=$USER_NAME" --data-urlencode "password=$USER_PASSWORD" --data-urlencode "requesttoken=$requestToken" "$SERVER_URL/index.php/login")

requestToken=$(extractRequestToken "$loginPost")

• Clear the APCu cache (call apcu_clear_cache() somehow, for example using https://github.com/krakjoe/apcu/blob/master/apc.php or the helper apps/testing/clean_apcu_cache.php added in this pull request; restarting the web server would reset the APCu cache, but it might also kill the user session and make the test invalid)

• In the same Bash terminal as before, do a request with the logged in user, for example:

curl --silent --location --cookie "/tmp/cookie-jar-$USER_NAME" --cookie-jar "/tmp/cookie-jar-$USER_NAME" --header "requesttoken: $requestToken" "$SERVER_URL/ocs/v1.php/cloud/user?format=json"

• Now request an endpoint in the weather_status app by a user member of the group that it is enabled for (again for simplicity admin is used here):

curl --user "$ADMIN_NAME:$ADMIN_PASSWORD" --header "OCS-ApiRequest: true" "$SERVER_URL/ocs/v2.php/apps/weather_status/api/v1/location"

Result with this pull request

The query succeeded

Result without this pull request

Invalid query returned; if the APCu cache is cleared again and the request repeated then it will now succeed (as the routes will be regenerated by the admin, which has access to the app)

[danxuliu]

/backport to stable32

[solracsf]

Is this still needed after #56926 ?

[provokateurin]

Yes the other PR was just a quick fix for the release. We still need to enable caching again and probably with this PR. @danxuliu can you rebase and revert my commit?

[provokateurin]

LGTM and also thanks for adding a test!

[danxuliu]

Rebased again to fix the DCO in the revert commit.

[come-nc]

I’m still wondering whether we should just load all enabled apps (for any user) even for users for which the application is not enabled.

The whole feature about enabling only for some users is a bit weird.

[nickvergessen]

Are we sure debug is false in this test, otherwise the 3s can be ran over between the requests and the cache already expired