Updated: Comment 0

Problem/Motivation

Most access checkers need the current account, as access is most of the time bound to the actual user.

Proposed resolution

Add an $account parameters to the access method, so the code would look like this:

Before

<?php
 
public function access(Route $route, Request $request) {
   
$account = $request->attributes->get('_account');
   
$permission = $route->getRequirement('_permission');
   
// @todo Replace user_access() with a correctly injected and session-using
    //   alternative.
    // If user_access() fails, return NULL to give other checks a chance.
   
return $account->hasPermission($permission) ? static::ALLOW : static::DENY;
  }
?>

After

<?php
 
public function access(Route $route, Request $request, AccountInterface $account) {
   
$permission = $route->getRequirement('_permission');
   
// @todo Replace user_access() with a correctly injected and session-using
    //   alternative.
    // If user_access() fails, return NULL to give other checks a chance.
   
return $account->hasPermission($permission) ? static::ALLOW : static::DENY;
  }
?>

Remaining tasks

User interface changes

API changes

Original report by @fubhy

This makes it much easier for our route access checks to properly use the currently logged in account. Now that we have it on the request we can easily forward it to our access checks.

Files: 
CommentFileSizeAuthor
#91 access-2048223-91.patch90.21 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 60,083 pass(es).
[ View ]
#89 access-2048223-89.patch90.75 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 59,249 pass(es).
[ View ]
#89 interdiff.txt680 bytesdawehner
#88 access-2048223-88.patch91.41 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 59,785 pass(es).
[ View ]
#88 interdiff.txt7.28 KBdawehner
#84 access-2048223-84.patch91.12 KBherom
PASSED: [[SimpleTest]]: [MySQL] 59,489 pass(es).
[ View ]
#84 interdiff-2048223-82-84.txt4.44 KBherom
#82 access-2048223-82.patch89.32 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 59,188 pass(es), 0 fail(s), and 280 exception(s).
[ View ]
#82 interdiff.txt10.55 KBdawehner
#80 2048223-80-account-accesscheckinterface.patch79.73 KBherom
FAILED: [[SimpleTest]]: [MySQL] 59,044 pass(es), 139 fail(s), and 287 exception(s).
[ View ]
#80 interdiff-2048223-78-80.txt2.75 KBherom
#78 2048223-78-account-accesscheckinterface.patch78.04 KBherom
FAILED: [[SimpleTest]]: [MySQL] 58,408 pass(es), 457 fail(s), and 2,301 exception(s).
[ View ]
#78 interdiff-2048223-75-78.txt4.11 KBherom
#75 2048223-75-account-accesscheckinterface.patch75.17 KBjoelpittet
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to enable simpletest module.
[ View ]
#70 interdiff.txt2.18 KBdawehner
#70 access-2048223-70.patch79.63 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch access-2048223-70_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#61 access_manager-2048223-61.patch77.45 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 58,782 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#61 interdiff.txt7.89 KBParisLiakos
#59 access_manager-2048223-59.patch74.58 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 58,788 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#59 interdiff.txt3.23 KBParisLiakos
#57 access_manager-2048223-57.patch72.36 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 58,531 pass(es), 4 fail(s), and 0 exception(s).
[ View ]
#57 interdiff.txt4.96 KBParisLiakos
#55 access_manager-2048223-55.patch68 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 59,002 pass(es), 90 fail(s), and 356 exception(s).
[ View ]
#55 interdiff.txt1.12 KBParisLiakos
#53 access_manager-2048223-53.patch67.36 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#53 interdiff.txt13.96 KBParisLiakos
#51 access_manager-2048223-51.patch53.4 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#49 access_manager-2048223-49.patch54.55 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 59,347 pass(es), 6 fail(s), and 0 exception(s).
[ View ]
#43 access_manager-2048223-43.patch55.49 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 58,066 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#43 interdiff.txt2.18 KBdawehner
#41 access_manager-2048223-41.patch55.16 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 58,080 pass(es), 2 fail(s), and 1 exception(s).
[ View ]
#38 access-2048223-38.patch57.5 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch access-2048223-38.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#38 interdiff.txt3.2 KBdawehner
#36 access-2048223-36.patch54.82 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]
#34 access-2048223-34.patch54.81 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch access-2048223-34.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#32 access-2048223-32.patch51.92 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch access-2048223-32.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#32 interdiff.txt15.18 KBdawehner
#30 access_manager-2048223-30.patch42.86 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 55,393 pass(es), 122 fail(s), and 24 exception(s).
[ View ]
#30 interdiff.txt1.65 KBdawehner
#28 access-account-2048223-28.patch42.7 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 57,703 pass(es), 72 fail(s), and 12 exception(s).
[ View ]
#28 interdiff.txt620 bytesdawehner
#26 access-2048223-26.patch42.68 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 6,996 pass(es), 940 fail(s), and 148 exception(s).
[ View ]
#26 interdiff.txt566 bytesdawehner
#23 access-2048223-23.patch42.4 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#23 interdiff.txt31.34 KBdawehner
#20 2048223-access-20.patch32.01 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 3,217 pass(es), 1,255 fail(s), and 88 exception(s).
[ View ]
#20 2048223-diff-18-20.txt594 bytesvijaycs85
#18 2048223-access-18.patch31.43 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 57,654 pass(es), 343 fail(s), and 1,049 exception(s).
[ View ]
#18 2048223-diff-16-18.txt481 bytesvijaycs85
#16 access-2048223-16.patch31.21 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 57,287 pass(es), 342 fail(s), and 1,049 exception(s).
[ View ]
#14 drupal-2048223-14.patch33.41 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]
#12 drupal-2048223-12.patch34.95 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]
#12 interdiff.txt580 bytesdawehner
#10 drupal-2048223-10.patch34.39 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]
#10 interdiff.txt0 bytesdawehner
#8 2048223-8.patch34.39 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]
#8 interdiff-2048223-8.txt1.09 KBdamiankloip
#6 2048223-6.patch33.29 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#6 interdiff-2048223-6.txt22.3 KBdamiankloip
#3 2048223-3.patch33.24 KBfubhy
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#1 2048223-1.patch34.32 KBfubhy
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new34.32 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

+++ b/core/modules/content_translation/content_translation.moduleundefined
@@ -337,7 +337,7 @@ function content_translation_edit_access(EntityInterface $entity, Language $lang
- *   The entity being translated.
+ *   The entity being translated.chx
  * @param \Drupal\Core\Language\Language $language
  *   (optional) The language of the translated values. Defaults to the current
  *   content language.
@@ -358,7 +358,7 @@ function content_translation_library_info() {
@@ -358,7 +358,7 @@ function content_translation_library_info() {
     'title' => 'Content translation UI',
     'version' => VERSION,
     'js' => array(
-      $path . '/content_translation.admin.js' => array(),
+      $path . '/conteant_translation.admin.js' => array(),
     ),
     'css' => array(

Hahaha, I guess I was trying to write on IRC when I wrote that patch :D

StatusFileSize
new33.24 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Re-uploading without the reference to chx :)

Status:Needs review» Needs work

The last submitted patch, 2048223-3.patch, failed testing.

Let's put the account parameter as the 3rd argument for the method, and make it optional (default NULL). I think that will make this change backward-compatible. (I agree it's a good chance, but we need to minimize breakage.)

Status:Needs work» Needs review
StatusFileSize
new22.3 KB
new33.29 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

I think I like that idea better, let's move that param.

Status:Needs review» Needs work

The last submitted patch, 2048223-6.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.09 KB
new34.39 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]

Status:Needs review» Needs work

The last submitted patch, 2048223-8.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new0 bytes
new34.39 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]

You just missed some places.

Status:Needs review» Needs work

The last submitted patch, drupal-2048223-10.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new580 bytes
new34.95 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]

For now we need this fix.

Status:Needs review» Needs work

The last submitted patch, drupal-2048223-12.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new33.41 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]

I explicitly removed the empty AccessInterface from the interface, so it is not an api change.

Status:Needs review» Needs work

The last submitted patch, drupal-2048223-14.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new31.21 KB
FAILED: [[SimpleTest]]: [MySQL] 57,287 pass(es), 342 fail(s), and 1,049 exception(s).
[ View ]

Rerolled.

Status:Needs review» Needs work

The last submitted patch, access-2048223-16.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new481 bytes
new31.43 KB
FAILED: [[SimpleTest]]: [MySQL] 57,654 pass(es), 343 fail(s), and 1,049 exception(s).
[ View ]

Re-roll + updating interface with $account.

Status:Needs review» Needs work

The last submitted patch, 2048223-access-18.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new594 bytes
new32.01 KB
FAILED: [[SimpleTest]]: [MySQL] 3,217 pass(es), 1,255 fail(s), and 88 exception(s).
[ View ]

seems this swap in arguments position fix most of the fails.

Status:Needs review» Needs work

The last submitted patch, 2048223-access-20.patch, failed testing.

The problem with the patch in #20 is that this is sadly an api change.

Title:Add $account argument to AccessCheckInterface::access() method.Add $account argument to AccessCheckInterface::access() method and use the curernt_user service
Status:Needs work» Needs review
StatusFileSize
new31.34 KB
new42.4 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Let's move the issue a bit.

After #2062151: Create a current user service to ensure that current account is always available we have a way to properly inject the account into the access manager.

Status:Needs review» Needs work

The last submitted patch, access-2048223-23.patch, failed testing.

Mh ...

Symfony\Component\DependencyInjection\Exception\ScopeWideningInjectionException: Scope Widening Injection detected: The definition "access_manager" references the service "current_user" which belongs to a narrower scope. Generally, it is safer to either move "access_manager" to scope "request" or alternatively rely on the provider pattern by injecting the container itself, and requesting the service "current_user" each time it is needed. In rare, special cases however that might not be necessary, then you can set the reference to strict=false to get rid of this error. in Symfony\Component\DependencyInjection\Compiler\CheckReferenceValidityPass->validateScope() (line 144 of /var/www/d8/core/vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/Compiler/CheckReferenceValidityPass.php).

Status:Needs work» Needs review
StatusFileSize
new566 bytes
new42.68 KB
FAILED: [[SimpleTest]]: [MySQL] 6,996 pass(es), 940 fail(s), and 148 exception(s).
[ View ]

There we go.

Status:Needs review» Needs work

The last submitted patch, access-2048223-26.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new620 bytes
new42.7 KB
FAILED: [[SimpleTest]]: [MySQL] 57,703 pass(es), 72 fail(s), and 12 exception(s).
[ View ]

This should fix most of the failures.

Status:Needs review» Needs work

The last submitted patch, access-account-2048223-28.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.65 KB
new42.86 KB
FAILED: [[SimpleTest]]: [MySQL] 55,393 pass(es), 122 fail(s), and 24 exception(s).
[ View ]

Sorry for all the noise, not sure whether this is nice, but I figured out why it actually fails:

Symfony\Component\DependencyInjection\ContainerBuilder->shareService(Object, Object, 'access_subscriber')
Symfony\Component\DependencyInjection\ContainerBuilder->createService(Object, 'access_subscriber')
Symfony\Component\DependencyInjection\ContainerBuilder->get('access_subscriber')
Symfony\Component\EventDispatcher\ContainerAwareEventDispatcher->lazyLoad('routing.route_alter')
Symfony\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('routing.route_alter', Object)
Drupal\Core\Routing\RouteBuilder->rebuild()
views_invalidate_cache()
Drupal\views\Entity\View->postSave(Object, 1)
Drupal\Core\Config\Entity\ConfigStorageController->save(Object)
Drupal\Core\Entity\Entity->save()
Drupal\block\Tests\Views\DisplayBlockTest->testDeleteBlockDisplay()

So the test code triggers are route rebuild, which gets the access subscriber, so the access manager which then relies on the current user.
Potentially splitting up the access manager or just make the current user optional might be a solution.

Status:Needs review» Needs work

The last submitted patch, access_manager-2048223-30.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+API change, +WSCCI, +Stalking Crell, +Routing
StatusFileSize
new15.18 KB
new51.92 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch access-2048223-32.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Splitted up the access subscriber from being dependent on the current user/request and one just handling the rebuilding.
Added the current user to the request aware event subscriber and pass it further to the access manager.

Status:Needs review» Needs work

The last submitted patch, access-2048223-32.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new54.81 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch access-2048223-34.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Missing file.

Status:Needs review» Needs work

The last submitted patch, access-2048223-34.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new54.82 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]

Just a rerole.

Status:Needs review» Needs work

The last submitted patch, access-2048223-36.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.2 KB
new57.5 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch access-2048223-38.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Let's see.

Status:Needs review» Needs work

The last submitted patch, access-2048223-38.patch, failed testing.

Title:Add $account argument to AccessCheckInterface::access() method and use the curernt_user serviceAdd $account argument to AccessCheckInterface::access() method and use the current_user service

Status:Needs work» Needs review
StatusFileSize
new55.16 KB
FAILED: [[SimpleTest]]: [MySQL] 58,080 pass(es), 2 fail(s), and 1 exception(s).
[ View ]

Here is a rerole.

------------
45e492dc05f38d697e9833fcef6bb53580aa7b94

Status:Needs review» Needs work

The last submitted patch, access_manager-2048223-41.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.18 KB
new55.49 KB
FAILED: [[SimpleTest]]: [MySQL] 58,066 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Let's upload the views fix.

Status:Needs review» Needs work

The last submitted patch, access_manager-2048223-43.patch, failed testing.

In EntityAccessController:

public function access(EntityInterface $entity, $operation, $langcode = Language::LANGCODE_DEFAULT, AccountInterface $account = NULL) {
public function createAccess($entity_bundle = NULL, AccountInterface $account = NULL, array $context = array()) {

This becomes strange, because $account is now always passed.

Right now we have to do $account = $this->prepareUser($account); at the beginning of each, which is just

  protected function prepareUser(AccountInterface $account = NULL) {
    if (!$account) {
      $account = $GLOBALS['user'];
    }
    return $account;
  }

this issue might provide a more generic solution to what we need to get config_translation into core. #2064697: Remove user_access from ConfigNameCheck

GRRR.

Status:Needs work» Needs review
StatusFileSize
new54.55 KB
FAILED: [[SimpleTest]]: [MySQL] 59,347 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

ok so TaxonomyTermCreateAccess moved to EntityCreateAccessCheck see #2068287: Support bundle names provided in the request arguments in EntityCreateAccessCheck

lets see how much i messed up the reroll

Status:Needs review» Needs work

The last submitted patch, access_manager-2048223-49.patch, failed testing.

Assigned:Unassigned» ParisLiakos
Status:Needs work» Needs review
StatusFileSize
new53.4 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

just a reroll
core/modules/block/lib/Drupal/block/Access/BlockThemeAccessCheck is dead thats why this is smaller

Status:Needs review» Needs work

The last submitted patch, access_manager-2048223-51.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new13.96 KB
new67.36 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Of course we have new access checkers. the drop moves!

Status:Needs review» Needs work

The last submitted patch, access_manager-2048223-53.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.12 KB
new68 KB
FAILED: [[SimpleTest]]: [MySQL] 59,002 pass(es), 90 fail(s), and 356 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, access_manager-2048223-55.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.96 KB
new72.36 KB
FAILED: [[SimpleTest]]: [MySQL] 58,531 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, access_manager-2048223-57.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.23 KB
new74.58 KB
FAILED: [[SimpleTest]]: [MySQL] 58,788 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

the HandlerFilterUserNameTest test passes locally:/

now..i believe the route enhancers we have for authentication...hmm dunno, they come pretty late obviously..
i ll check tomorrow

this should fix at least 1 test

Status:Needs review» Needs work

The last submitted patch, access_manager-2048223-59.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new7.89 KB
new77.45 KB
FAILED: [[SimpleTest]]: [MySQL] 58,782 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

synchronized FTW

Status:Needs review» Needs work

The last submitted patch, access_manager-2048223-61.patch, failed testing.

Assigned:ParisLiakos» Unassigned

the remaining failures, pass locally, so should be php5.3/enviroment specific errors

wow! Just wondering why the same tests failed before but we need all the synchronized services for this issue. I totally agree that it is AWESOME that you found
all these bugs

yes i agree its awesome:D

#59 was failing in access tests
in #61 those are fixed cause of the changes in the access subscriber, but we got two new in Drupal\block\Tests\Views\DisplayBlockTest

i ll ask for a re-test in case some of them where random.

about synced: i think we should do the same for the request, but in a different issue;)

Status:Needs work» Needs review
Issue tags:-Needs issue summary update, -API change, -WSCCI, -Stalking Crell, -Approved API change, -Routing

#61: access_manager-2048223-61.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Needs issue summary update, +API change, +WSCCI, +Stalking Crell, +Approved API change, +Routing

The last submitted patch, access_manager-2048223-61.patch, failed testing.

+++ b/core/core.services.yml
@@ -177,7 +177,8 @@ services:
   plugin.manager.menu.local_task:
     class: Drupal\Core\Menu\LocalTaskManager
-    arguments: ['@controller_resolver', '@request', '@router.route_provider', '@module_handler', '@cache.cache', '@language_manager', '@access_manager']
+    arguments: ['@controller_resolver', '@request', '@router.route_provider', '@module_handler', '@cache.cache', '@language_manager', '@access_manager', '@current_user']
+    scope: request

Urgs, I would really like to avoid this if possible.

it just says the truth:) i dont see whats bad about it. it depends on the request, so it is in request scope

Status:Needs work» Needs review
StatusFileSize
new79.63 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch access-2048223-70_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new2.18 KB

This test is pretty dump, so maybe this works ... but well

Status:Needs review» Needs work
Issue tags:-Needs issue summary update, -API change, -WSCCI, -Stalking Crell, -Approved API change, -Routing

The last submitted patch, access-2048223-70.patch, failed testing.

Status:Needs work» Needs review

#70: access-2048223-70.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Needs issue summary update, +API change, +WSCCI, +Stalking Crell, +Approved API change, +Routing

The last submitted patch, access-2048223-70.patch, failed testing.

This is blocked on doing something sane for #2095125: Use access constants in every access control context

Status:Needs work» Needs review
Issue tags:-WSCCI, -Stalking Crell, -Routing
StatusFileSize
new75.17 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to enable simpletest module.
[ View ]

Re-rolled #70

Issue tags:+WSCCI, +Stalking Crell, +Routing

adding tags back, wtf

Status:Needs review» Needs work

The last submitted patch, 2048223-75-account-accesscheckinterface.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.11 KB
new78.04 KB
FAILED: [[SimpleTest]]: [MySQL] 58,408 pass(es), 457 fail(s), and 2,301 exception(s).
[ View ]

get #75 past the install fails. also fix some Drupal → \Drupal cases.

Status:Needs review» Needs work

The last submitted patch, 2048223-78-account-accesscheckinterface.patch, failed testing.

Assigned:Unassigned» herom
Status:Needs work» Needs review
StatusFileSize
new2.75 KB
new79.73 KB
FAILED: [[SimpleTest]]: [MySQL] 59,044 pass(es), 139 fail(s), and 287 exception(s).
[ View ]

patch update.

Status:Needs review» Needs work

The last submitted patch, 2048223-80-account-accesscheckinterface.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new10.55 KB
new89.32 KB
FAILED: [[SimpleTest]]: [MySQL] 59,188 pass(es), 0 fail(s), and 280 exception(s).
[ View ]

Somes fixes here and there.

Status:Needs review» Needs work

The last submitted patch, access-2048223-82.patch, failed testing.

Assigned:herom» Unassigned
Status:Needs work» Needs review
StatusFileSize
new4.44 KB
new91.12 KB
PASSED: [[SimpleTest]]: [MySQL] 59,489 pass(es).
[ View ]

another update. hoping for green.

hey its green!

  1. +++ b/core/modules/aggregator/lib/Drupal/aggregator/Access/CategoriesAccessCheck.php
    @@ -44,10 +45,10 @@ public function appliesTo() {
         // @todo Replace user_access() with a correctly injected and session-using
         // alternative.

    lets remove this comment too

  2. +++ b/core/modules/user/lib/Drupal/user/Tests/Views/HandlerFilterUserNameTest.php
    @@ -84,20 +84,17 @@ protected function setUp() {
    -    // Test all of the accounts with a single entry.
    ...
    -    foreach ($this->accounts as $account) {
    -      $view->filter['uid']->value = array($account->id());
    -    }
    +    $view->filter['uid']->value = array($this->accounts[0]->id());
    ...
    -    $this->assertIdenticalResultset($view, array(array('uid' => $account->id())), $this->columnMap);
    +    $this->assertIdenticalResultset($view, array(array('uid' => $this->accounts[0]->id())), $this->columnMap);
    ...
    -  public function testAdminUserInterface() {
    +  public function ptestAdminUserInterface() {
    @@ -140,7 +137,7 @@ public function testAdminUserInterface() {
    -  public function testExposedFilter() {
    +  public function ptestExposedFilter() {
    +++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/InOperator.php
    @@ -384,6 +384,7 @@ protected function opSimple() {
    +    debug($this->value);

    So lets revert those chnages now..unless the test ones are actually needed?

  3. +++ b/core/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php
    @@ -40,6 +47,7 @@ public static function getInfo() {
    +    $this->account = $this->getMock('Drupal\Core\Session\AccountInterface');
    +++ b/core/tests/Drupal/Tests/Core/Entity/EntityAccessCheckTest.php
    @@ -50,7 +50,8 @@ public function testAccess() {
    +    $account = $this->getMock('Drupal\Core\Session\AccountInterface');
    +++ b/core/tests/Drupal/Tests/Core/Entity/EntityCreateAccessCheckTest.php
    @@ -118,7 +118,8 @@ public function testAccess($entity_bundle, $requirement, $access, $expected) {
    +    $account = $this->getMock('Drupal\Core\Session\AccountInterface');
    +++ b/core/tests/Drupal/Tests/UnitTestCase.php
    @@ -181,4 +181,13 @@ public function getStringTranslationStub() {
    +  public function getAccountStub() {

    So i see we got a getAccountStub method, although its more of a mock:P
    I am not sure it makes much sense to have a method here?
    But anyway we should update those calls, to call the method instead

I don't see how getAccountStub is useful. It's a one liner, just mock the interface you want.

I don't see how getAccountStub is useful. It's a one liner, just mock the interface you want.

Well, the reason I added it was because this is potentially needed in a hell lot of places.

StatusFileSize
new7.28 KB
new91.41 KB
PASSED: [[SimpleTest]]: [MySQL] 59,785 pass(es).
[ View ]

So lets revert those changes now..unless the test ones are actually needed?

It is needed to fix the failures. If you have a look at the existing code you really wonder how this could have ever passed.
It did not worked on purpose and got broken somehow by that patch.

Fixed the other stuff.

StatusFileSize
new680 bytes
new90.75 KB
PASSED: [[SimpleTest]]: [MySQL] 59,249 pass(es).
[ View ]

No debug().

Status:Needs review» Reviewed & tested by the community

+++ b/core/core.services.yml
@@ -631,6 +642,7 @@ services:
+    synchronized: true
+++ b/core/lib/Drupal/Core/DependencyInjection/YamlFileLoader.php
@@ -107,6 +107,10 @@ protected function parseDefinition($id, $service, $filename) {
+    if (isset($service['synchronized'])) {
+      $definition->setSynchronized($service['synchronized']);
+    }

Nice!

This looks ready to go.

StatusFileSize
new90.21 KB
PASSED: [[SimpleTest]]: [MySQL] 60,083 pass(es).
[ View ]

_search_menu_access() was removed, so this needed a reroll.

+++ b/core/core.services.yml
@@ -192,10 +192,11 @@ services:
+    scope: request

Why is the request scope added here, when we had to remove it from the current user service in #2076411: Remove the request scope from the current user service? We have a critical to add that back, would just like to know why it breaks without it or whether this is a nicety.

Otherwise looks great.

i added it before #2076411: Remove the request scope from the current user service went in. now its not needed, but we wiill have to add it back when current_user get under request scope again. so i think just leaving it there its ok

Hmm OK. We have that critical to add it back, so it'll all make sense again once that happens.

Patch doesn't apply any more unfortunately.

#91: access-2048223-91.patch queued for re-testing.

Patch seemed to apply here still.

Title:Add $account argument to AccessCheckInterface::access() method and use the current_user serviceChange notice: Add $account argument to AccessCheckInterface::access() method and use the current_user service
Priority:Critical» Major
Status:Reviewed & tested by the community» Active

Hmm yes my mistake locally..

Committed/pushed to 8.x, thanks! Will need a change notice.

Issue summary:View changes

todo

Issue summary:View changes
Issue tags:-Avoid commit conflicts

Status:Active» Needs review
Issue tags:+Needs change record

@dawehner, I guess this is "Needs review" for the change record updates you list? Do we need to add anything else?

Status:Needs review» Fixed

I think we already cover now all in the existing change notifications.

Title:Change notice: Add $account argument to AccessCheckInterface::access() method and use the current_user serviceAdd $account argument to AccessCheckInterface::access() method and use the current_user service
Issue tags:-Needs change record

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.