Add Entity Access check for valid bundle names [#1987550]

Follow up for #1971384: [META] Convert page callbacks to controllers

Problem/Motivation

We need to validate entity bundle names in #1978880: Convert field_test_entity_add to a new style controller and possibly others such as node/add/%type and #1978166: Convert block/add/{%custom_block_type} to Controller

Proposed resolution

Add an entity access check to validate that a bundle name in a slug is in fact valid.

Remaining tasks

Reviews

Comment	File	Size	Author
	entity-bundle-access.patch	5.6 KB	larowlan

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

larowlan

🇦🇺🏝.au GMT+10

CreditAttribution: larowlan commented 6 May 2013 at 07:43

Do we want to make the slug name variable eg node.bundle_name being entity_type.slug_name
At present we're assuming its bundle_name in the slug.

Comment #2

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett commented 6 May 2013 at 07:46

I'm not sure why this is checking the entity manager, bundles were moved to hook_entity_bundle_info() and entity_get_bundles().

Comment #3

dawehner

German

CreditAttribution: dawehner commented 6 May 2013 at 08:57

Maybe it makes sense to introduce a validation layer. Views could certainly use that and I guess scotch would be a perfect candidate for it as well,
though I'm not sure whether using the access checking is conceptually the right thing to do.

Comment #4

larowlan

🇦🇺🏝.au GMT+10

CreditAttribution: larowlan commented 6 May 2013 at 10:58

Its using the entity manager so I can mock it in the unit test.

Comment #5

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett commented 6 May 2013 at 13:53

I'm saying, no matter how many times you check the entity manager, it will not have the bundle information.

Comment #6

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett commented 6 May 2013 at 15:35

This is scary, that you can write a unit test making a service return something it never will O_o

See #1822458: Move dynamic parts (view modes, bundles) out of entity_info()


+++ b/core/tests/Drupal/Tests/Core/Entity/EntityBundleAccessCheckTest.phpundefined
@@ -0,0 +1,91 @@
+      ->will($this->returnValue(array('node' => array('foo' => array()))));

This isn't even the return signature the checker is looking for...

Comment #7

larowlan

🇦🇺🏝.au GMT+10

CreditAttribution: larowlan commented 6 May 2013 at 21:35

Status:

Needs review

» Closed (won't fix)

So @timplunkett rightly points out that ::getDefinitions no longer contains bundle info, not sure where I imagined that from.
And @dawehner points out that Access denied isn't the way to go here, so we need a better solution.
In terms of #1978880: Convert field_test_entity_add to a new style controller I think that should be returning a not found exception from the controller callback if the bundle name is invalid.