- Issue created by @ananthakrishnan.kr
- 🇮🇳India balramchauhan684@gmail.com
public function getCacheSuffix() {
return 'key_config_override';
}Try changing return value of getCacheSuffix function and test it again.
- 🇮🇳India ananthakrishnan.kr
I tried changing return value of getCacheSuffix function. But still we have the issue.
- 🇳🇿New Zealand quietone
Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.
Can you execute a
git bisect
on a Git working copy of Drupal Core between 10.2.7 and 10.3.0 to quickly find the commit that changed this behavior?- 🇮🇳India nitesh624 Ranchi, India
will git bisect show the code difference between these two versions?
- 🇮🇳India nitesh624 Ranchi, India
I ran the bisect between tags 10.2.7 and 10.3.1 by running below command
git bisect start
git bisect good tags/10.2.7
git bisect bad tags/10.3.1then the commit which I get is setting the drupal core to 11.0-dev which leads to break testing.
- 🇮🇳India nitesh624 Ranchi, India
This https://git.drupalcode.org/project/drupal/-/commit/9bb46296cee3aa33a9750... is the commit on Mar 27, 2024 which causing our test to failure.
- 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium
We never touched config overrides in that issue. The only thing that has changed is that we cache the outcome of your permission calculations, including the ones coming from roles, because that's the only way to safely do so.
Altering permissions live is a security risk, the code in the IS is unsafe. I specifically designed the Access Policy API because I too needed alterable permissions but found out that dynamically changing what permissions a role has is an attack vector. I shall not go into further detail on that matter because you apparently have such code in your system.
My advice would be to introduce an access policy to take care of whatever logic you had in your config overrides. Then it will work and be safe.
I'm actually having a similar problem in my project...
We have some functionality that allows an anonymous user to view an unpublished page via a URL hash. We have an existing functional test that basically visits an unpublished page (say
node/1
) and we check for a 403. Then we visit the page again with the hash as part of the URLnode/1?hash=validHashHere
and check for a 200. We are now receiving a 403 after the 10.3.1 update.It seems that the above commit in question definitely has something to do with it (especially with the addition of
permissionsHashGenerator
inAccountPermissionsCacheContext.php
). Still debugging though...- 🇮🇳India nitesh624 Ranchi, India
I have update my custom code to assign permission through access policy but facing same issue
- 🇮🇳India nitesh624 Ranchi, India
Here is my code snippet
-> custom_access/src/Access/AccessUnpublishedPermission.php
<?php namespace Drupal\custom_access\Access; use Drupal\Core\Session\AccessPolicyBase; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Session\CalculatedPermissionsItem; use Drupal\Core\Session\RefinableCalculatedPermissionsInterface; /** * Using the access policy API to allow users to view unpublished content. */ class AccessUnpublishedPermission extends AccessPolicyBase { /** * {@inheritdoc} */ public function calculatePermissions(AccountInterface $account, string $scope): RefinableCalculatedPermissionsInterface { $calculated_permissions = parent::calculatePermissions($account, $scope); $types = \Drupal::entityTypeManager()->getStorage('node_type')->loadMultiple(); $permission = []; foreach ($types as $type) { $permission[] = 'access_unpublished node ' . $type->id(); } // If we aren't an anonymous user, return early. if (!in_array('anonymous', $account->getRoles())) { return $calculated_permissions; } return $calculated_permissions->addItem(new CalculatedPermissionsItem($permission)); } public function getPersistentCacheContexts(): array { return ['user.roles:anonymous']; } }
-> custom_access.services.yml
services: custom_access.access_policy.access_unpublished_permission: class: Drupal\custom_access\Access\AccessUnpublishedPermission tags: - { name: access_policy }
Then in Functional Test I am creating a test content type like below
$this->drupalCreateContentType(['type' => 'test_content', 'name' => 'test_content']);
The problem is after creating the content type like this in test,
\Drupal::entityTypeManager()->getStorage('node_type')->loadMultiple();
service inAccessUnpublishedPermission.php
file does not include thetest_content
content type - 🇮🇳India nitesh624 Ranchi, India
I agreed with https://www.drupal.org/project/drupal/issues/3463722#comment-15717005 🐛 ConfigFactoryOverrideInterface::loadOverrides not invoked after content type creation in Drupal 10.3.0+ Active . But will the caching changes affect the outcome of
$types = \Drupal::entityTypeManager()->getStorage('node_type')->loadMultiple();
in above code snippet, when we are creating node in a Functional test like$this->drupalCreateContentType(['type' => 'test_content', 'name' => 'test_content']);
? - 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium
No, loading node types should still work
- 🇮🇳India nitesh624 Ranchi, India
But in my case newly created node
Type in functional test is not coming in config override service, Unless we forcefully clear the cache in functional test.So somthing cache related in permission policy api causing this behaviour in our drupal site
- 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium
I cannot judge this properly because I have no overview of your custom code. That said, the tests in core clearly show that node retrieval does work after the implementation of the access policy API. The cache in the access policy API only caches your calculated permissions, nothing else.
- 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium
But I already explained why that piece of code won't work and is dangerous. Try your access policy, and only that access policy, in a clean Drupal install and see if things are still broken. The access policy API does not affect node loading, so clearly something is going on here.
- 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium
Oh hold up, I get it now. You forgot to set the cacheability. You rely on the list of node types, but never mention that anywhere. So of course you are getting the cached result of the previously configured node types.
- 🇮🇳India nitesh624 Ranchi, India
Yes with access policy approach we were able to get that working after adding cacheability .
But I am asking about the config override approach (below code)
class PermissionOverrides implements ConfigFactoryOverrideInterface { /** * {@inheritdoc} */ public function loadOverrides($names) { $overrides = []; $role = 'user.role.test_role'; if (in_array($role, $names)) { $types = $this->entityTypeManager()->getStorage('node_type')->loadMultiple(); foreach ($types as $type) { $permission = 'Test permission ' . $type->id(); $overrides[$role]['permissions'][$permission] = $permission; } } return $overrides; } /** * @return string */ public function getCacheSuffix() { return static::class; } /** * @param string $name * * @return \Drupal\Core\Cache\CacheableMetadata */ public function getCacheableMetadata($name) { return new CacheableMetadata(); } /** * @param mixed $name * @param string $collection * * @return \Drupal\Core\Config\StorableConfigBase|null */ public function createConfigObject($name, $collection = StorageInterface::DEFAULT_COLLECTION) { return NULL; } }
Code snippets for Content type creation in Functional Test.
$this->drupalCreateContentType(['type' => 'test_content', 'name' => 'test_content']);
The above code is not working after inclusion of permission policy api. Means the
test_content
content type which we are creating in Functional test is not coming in the$types = $this->entityTypeManager()->getStorage('node_type')->loadMultiple()
;When I debug more on this I found that on
web/core/lib/Drupal/Core/Session/PermissionChecker.php
file underhasPermission()
, When I reverted back the changes committed in https://git.drupalcode.org/project/drupal/-/commit/9bb46296cee3aa33a9750... commit, Then my config override method is working fine - 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium
Yes with access policy approach we were able to get that working after adding cacheability . But I am asking about the config override approach.
I already told you in #10 that that is unsafe.
I gave you a solution, you told me it works, so why not use that? Also at this point this is probably better categorized as a support request.