Let's add an API for checking access to entity objects, e.g. like the entity_access()
function of the d7 contrib entity API module. Furthermore, the API should provide means for checking property/field access based upon #1346214: [meta] Unified Entity Field API, while incorporating entity access.
Note, that this issue is supposed to implement access checks on already loaded entity objects and/or checking general access, but not about applying access checks on queries. Let's handle that in another issue, e.g. #777578: Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter().
Problem / Motivation
For implementing web services an easy way to check entity access is necessary, such that inaccessible properties can be filtered out and access to inaccessible entities is denied. Of course, that API would be useful in many other situations as well, i.e. for pretty much every module that wants to generally support entities and incorporate access checks.
Implementation plan
- Work in the lines of #1302378: Use multiple specialized entity controllers, i.e. implement a sane default behaviour in a swappable and customizable controller class.
- Keep it optional: entity types may specify an access controller, but it's not mandatory.
Related issues
#1810320: Remove EntityTranslationControllerInterface::getAccess() once have entity access is postponed on this issue.
Follow-up issues
#1825332: Introduce an AccountInterface to represent the current user
Comment | File | Size | Author |
---|---|---|---|
#111 | 1696660-111.patch | 19.17 KB | fubhy |
#110 | entity-1696660-110.patch | 19.2 KB | xjm |
#110 | interdiff-110.txt | 1 KB | xjm |
#102 | entity-1696660-100.patch | 19.2 KB | xjm |
#102 | interdiff-100.txt | 1 KB | xjm |
Comments
Comment #0.0
fagoUpdated issue summary.
Comment #1
sunComment #2
Leeteq CreditAttribution: Leeteq commentedRelated discussion in #1704168: Move node access functions to separate module (closed as duplicate in favor of this issue)
Comment #3
larowlantagging
Comment #4
larowlanThis is needed for #731724: Convert comment settings into a field to make them work with CMI and non-node entities, tagging as Platform Initiative accordingly
Comment #5
fubhy CreditAttribution: fubhy commentedDid anything happen during that sprint @larowlan? Otherwise I am going to start working on this.
Comment #6
larowlanFubhy, go for it - pnx-sprint is a weekly mini-sprint.
Comment #7
YesCT CreditAttribution: YesCT commented#1810320: Remove EntityTranslationControllerInterface::getAccess() once have entity access is postponed on this issue. (updated issue summary)
Comment #7.0
YesCT CreditAttribution: YesCT commentedUpdated issue summary.
Comment #8
Wim LeersSpark's Edit module D8 port is also blocked on this.
(I tried duplicating
entity_access()
just for Edit for now, but core doesn't yet implement the necessary access callbacks and I didn't feel like duplicating all that. Hence, I currently am skipping permissions checking until this is ready.)Comment #9
Wim LeersTagging.
Comment #10
fubhy CreditAttribution: fubhy commentedOkay. So the first step for me would be to implement access checks for loaded entity objects.
So... regarding "any" access ("general" access): I was not entirely sure how we should implement that or if it is even part of this issue. AnyAccess checks work in such a custom way across core and contrib (does the langcode matter? does the bundle matter? etc.) that I wasn't really sure how to reflect that in the controller.
I started with something like this:
But fago had some objections. So I am leaving this open for discussion for now.
Comment #11
fubhy CreditAttribution: fubhy commentedComment #12
Damien Tournoud CreditAttribution: Damien Tournoud commentedI would avoid calling the method
$operation
directly. I would recommend${operation}Access
, so that we can safely add methods to the access controllers later on.Comment #13
fagoLet's better return NULL in this case. For a lot of module TRUE is a good default, but not for all. With NULL, modules can decide upon the default theirselves.
I'd call that "AccessControllerBase". With namespaces we don'T necessaryil need the Entity prefix any more, see DataBaseStorageController.
Should describe how it canbe used to support unknown operations.
Can we have that default to the default entity language? As default value, we can use the LANGUAGE_DEFAULT constant.
typo
Should say, that if it's NULL the current user is used.
Calling it with an unsupported operation? I'd expect an InvalidArgumentException - not a valid return value.
I'm not sure we really need a separate test case for that. Why not just move it over to entity access tests and have everything in one place?
What about the idea of having that in the access controller?
E.g. have a method
hook_permission() and invoke it from system.module for all access controllers. Then, an access controller can define and test it's own access permissions.
Misses docs.
I don't think so. Probably it even shouldn't be part of the base controller. For defining support for additional $ops one might want to implement a proper method anyway. Imo, it suffices to document at the access controller interface how custom operations can be supported.
Indeed.
Yep. Before introducing lots of different combinations, I think we'd have to research what's really used somewhere, that's definitely a follow-up though. I could see us already adding general All access, but I'm fine with postponing that as well.
Comment #14
fubhy CreditAttribution: fubhy commentedOkay, agreed.
Entirely removed for now.
Same.
Agreed.
Hmm... It does?! :)
Yes... I had much more stuff there before I removed the "general" access bits. And yes... let's keep them separated for now. Without them this patch is actually really simple and small.
I am not sure. In many cases entity access does not have explicit entity-related permissions but instead uses permissions that e.g. other modules provide or one that is not limited to the entity. Also, that would mean that we would have to use system module for it which I really don't like :(.
Right... done.
Comment #15
klausiIMO, this should default to FALSE if there is no access controller. I cannot think of an entity type that would allow view and edit for arbitrary users. So TRUE is not really a sensible default.
Comment #16
fubhy CreditAttribution: fubhy commentedHmm... Not sure about that. I think that really is a question of how we define it. Let's see what others think about that.
Comment #17
fubhy CreditAttribution: fubhy commentedOkay... I am on board now with defaulting to FALSE. That indeed is a much more sensible default. Thanks klausi. I also did some further cleanup (still had some leftovers from the more complex implementation left). This patch now basically just defines the interface and how it talks to entities and entity translations. Everything else should be postponed to follow-ups as this issue currently blocks quite a few others.
Comment #18
fagoI thought you agreed to having it default to NULL? :-) As said, the reasonable default depends on the use case - so let modules treat the case as they want.
Comment #19
fubhy CreditAttribution: fubhy commentedlol... misunderstanding there :). Okay.. another re-roll then:
Comment #20
klausiThat violates the contract of AccessibleInterface::access(). TRUE or FALSE, what's it gonna be?
Comment #21
pounardDeny all and allow on a per use case basis as a default for all access operations sounds security wise. I'd recommend FALSE here. But, this is a special case since there is no access controller: it's matter of do we decide there will always be an access controller? Case in which we need to enforce a default access controller as fallback and not treat this case that should not happen. Not having an access controller sounds like a pragmatic error that should throw an exception, thus leaving the code in a consistent state where we cannot not use an access control for entities. What do you think?
Comment #22
sun"No" is the answer you get for all security questions by default.
AFAIK, there is no entity in core that has absolutely zero access conditions, at minimum there's a user permission. The edge-case entity that really has none can implement a custom NullAccessController.
Comment #23
pounardI think we agree, so this:
Should become this:
Comment #24
fubhy CreditAttribution: fubhy commentedOkay. Adding a defualt controller as suggested by @pounard. This might look like it is redundant because it simply returns FALSE all the time but at the same time it gives us consistency for the return value of ->access and we don't have to implement custom behaviors there everywhere we use entity_access_controller() somewhere. Plus, custom entities only have to implement the methjods that they want to support in a custom way.
Comment #25
fubhy CreditAttribution: fubhy commentedMoving the default controller definition to entity_get_info().
Also @see #1821662: Register entity controllers with the DIC.
Comment #26
Lars Toomre CreditAttribution: Lars Toomre commentedSmall nit if this gets re-rolled... This should have an initial slash '\' according to #1487760: [policy, no patch] Decide on documentation standards for namespaced items.
Comment #27
Lars Toomre CreditAttribution: Lars Toomre commentedWhoops... Did not mean to change status.
Comment #28
fagoShould mention that it defaults to access being FALSE.
class should tell what it is: e.g. call it
DefaultAccessController ? or better NoAccessController ?
Nitpicking, but it defaults to NULL, what means the current user.
"+ * (optional) The user for whom to check 'view' access for or NULL to check access for the current user. Defaults to NULL.
Comment #29
sunTurning it into the default access controller makes sense. I'd imagine that it could even be the base controller for all others, so anything that isn't overridden defaults to FALSE.
That said, I see CRUD in there, but reality is CRUDL. I think it would be wise to add a listAccess() method right from the start.
Comment #30
fubhy CreditAttribution: fubhy commented@sun I book that under "AnyAccess" methods (I would not call it list access, I think that term rather describes "query access"). Those need custom arguments that are different to those used by our single "loaded" object access checks. Just look at node_access for example. We would also need an optional $bundle argument there. And I think we can find quite a few more exceptions. In the end we will have to define an interface that would be able to solve most of those custom use-cases but it would take a considerable amount of bikeshedding time to figure that out. Considering that this issue currently blocks others and the patch is really just a super simple foundation for further entity access issues we should wrap this up first before we discuss any other features that we need to cover.
That said, in the end we will not only need something like CRUDL (LIST) we basically need AnyAccess check capabilities for CREATE, UPDATE, DELETE, etc. - We might even want to discuss if we want a global access check that would out-rule all others (accessByPass() or something). But, again, I think we should do that stuff in a follow-up.
Comment #31
pounardL is not an entity permission, but the aggregation of successive R operations over a set of entities. I don't think it makes sense to integrate it into the access controller, this check belongs at query time, when listing.
Comment #32
fagoI could see us making an exception for "all-access", but generally - yes.
Agreed!
Comment #33
fubhy CreditAttribution: fubhy commentedApart from renaming the default controller (all the other controllers / base controllers are prefixed with Entity too currently and I don't want to make the first step) I believe I did everything that was asked for.
Comment #34
Lars Toomre CreditAttribution: Lars Toomre commentedHere are some comments from reading through the patch in #33.
This needs to be a fully qualified path to EntityInterface. Same below.
Can these be sorted alphabetically?
Can we change $msg to $message? Also, there is not need to use t() here. Use format_string() instead.
Also, for readability, it would help to breakout the array elements on separate lines.
It would be helpful not reuse the $user variable here. Perhaps call this $custom_user instead?
Why not use the state() system instead of variable_set()?
Comment #35
Lars Toomre CreditAttribution: Lars Toomre commentedDid not mean to change status...
Comment #36
fubhy CreditAttribution: fubhy commentedNext try.
Edit:
Oh, good catch. That was copied over from the NodeAccess tests.
I don't think we have an actual standard for that yet: #1624564: Coding standards for "use" statements
But sure, I can reorganize those.
Okay
This is part of the entity_test module and the way that the translation tests trigger the stuff they inject in their mock modules. I am just using it too!
Comment #37
Lars Toomre CreditAttribution: Lars Toomre commentedThanks @fubhy! I presume that there is an issue somewhere to convert those variable_set()'s. We should add a note to that one so that they are aware that we are using the variable here too.
Comment #39
fubhy CreditAttribution: fubhy commented#36: 1696660-36.patch queued for re-testing.
Comment #40
fagoTechnically, the patch looks good, however it misses docs on the possible values for $op in the accessibleinterface as well as on the entityinterface level. Also, the $user docs got only updated for viewAccess. I fixed that and added some docs for $op.
Note, that we cannot redefine access() on the EntityInterface as this results in a fatal (at least on my install), maybe it's working with php 5.3.10. See related comment: http://drupal.org/node/1800122#comment-6630376
Updated patch attached.
Comment #41
sunThis looks good to me. Only a small, but important nit:
Everywhere in this patch:
The user account should always be in an $account variable, not $user, so as to ensure that it never clashes with global $user.
Comment #42
fubhy CreditAttribution: fubhy commentedOkay. This should be good to go then!
Comment #43
sunThanks!
This patch looks fine to me. It makes a good, basic step towards a more sophisticated entity access API.
There's a lot of work ahead there, so I think it makes sense to move forward here and to hash out more granular details in the follow-up issues.
A unified entity access API is very badly needed. It should have been in D7 already, and the fact that it wasn't made the entity system considerably harder to work with. Therefore, fixing this gap is critically important for D8 from my perspective.
(At some point in the process, I want and need to make sure that special use-cases of modules like Mollom are taken care of, which act on all entity types, and also, which sometimes need to figure out "reverse access permissions" which seem to be foreseen as bypass permissions in here. But yeah, laters.)
Comment #44
plachEeew. We need at very least a @todo about fixing the
AccessibleInterface
and remove this hack. This is really bad DX.These are violating the requirement stating that code in the
Drupal\Core
namespace may not depend on code provided by a module. I am afraid we need aDrupal\Core\AccountInterface
(or somesuch) thatDrupal\user\User
implements.Also, why aren't we using the
use
statement here?Comment #45
fagoWell, having different $ops is not something invented here, we already use 'view' and 'edit' for field access while we use 'create', 'update', 'delete' and 'view' for entity access. I'm not sure it's such a bad hack - it's just mapping between our existing ops. I agree that's unfortunate, but how it is now. We could discuss unifying $ops in a separate issue, but how create/update/delete would apply to a field is questionable.
True, but that's not invented by this patch either. I'm not sure what's the best way to handle this, but I think we should care about that in its own issue.
Comment #46
plachWe can totally defer this, that's why I suggested a @todo. IMHO the pain point is only that IIUC we are using 'edit' and 'update' to indicate the same op, albeit in different contexts.
If we remove those FQCN, what about a @todo above the related use statement?
Comment #47
fubhy CreditAttribution: fubhy commented'Update' is a type of 'edit' operation with the current meaning of 'edit' (for fields, for example, 'edit' covers 'create', 'update' as well as 'delete' operations).
I added the @todo's to both interfaces (EntityAccessControllerInterface and AccessibleInterface).
But yes, that user module dependency was not introduced here and should be fixed somewhere else too. I am opening a follow-up for that.
Edit: Oh, that interdiff is in reverse. But you get the idea... :)
Comment #48
fubhy CreditAttribution: fubhy commented#1825332: Introduce an AccountInterface to represent the current user
Comment #49
fubhy CreditAttribution: fubhy commentedAnother follow-up: #1825346: Introduce an access wrapper function for use in menu system access callbacks, etc.
Comment #50
fubhy CreditAttribution: fubhy commentedSo... It's green, we got the @todo's in place and the follow-up issue have been created. Resetting this to RTBC. Let's proceed in the follow-ups.
Will write the change notice once it's committed.
Comment #51
sunCollecting Entity Access issues:
http://drupal.org/project/issues/search/drupal?issue_tags=Entity%20Access
Comment #52
plach@fubhy:
Thanks!
Comment #53
fagoI agree this is good to go.
Comment #54
xjm#47: 1696660-47.patch queued for re-testing.
Comment #56
fubhy CreditAttribution: fubhy commentedRe-roll. Also removed entity_access_test mock module in favor and put that stuff into entity_test directly.
Comment #57
Stalski CreditAttribution: Stalski commentedLooking good to me
Comment #58
catchLooks fine to me although I haven't done a full review yet.
It's a simple enough patch but that's a good thing in this case. Couple of questions?
Why? Can't we just change the calling code to use 'update'? Also it's a bit odd with the $op argument, why not four methods like the access interface?
Comment #59
fubhy CreditAttribution: fubhy commentedWe need a single entry point. This implements the AccessibleInterface which excepts 'edit' and 'view' as $op. We simply extend that but since entity access checks have more granular permissions we have to do this mapping.
Comment #60
fagoIt can. This is just to be consistent with the AccessibleInterface, which defines 'view' and 'edit' as used for fields. Imho the docs are unnecessary complicated becaues of that, so a little wrapper as suggested in #1825346: Introduce an access wrapper function for use in menu system access callbacks, etc. could help that as well as redefining the access() method in the entity interface, once we required 5.3.10 (see #40)
Should we include the entity_access() wrapper with this patch to ease things right now?
Maybe, but it might be even useful to have $op in some cases - e.g. if your general edit page just has a $op that differentaties between create and update. Also I think this goes more inline which we had before as node_access() (and contrib entity_access() had it that way, so I'd prefer keeping it this way right now.
Setting back to RTBC for getting committer feedback.
Comment #61
chx CreditAttribution: chx commentedWe have an existing node access system and I have proposed elevating it to entity access in #1819726: Move node access code into a pluggable class
Edit: this is off topic, I misunderstood the intent of this patch. The next comment fixes the misunderstanding by retitleing the issue.
Comment #62
chx CreditAttribution: chx commented{"{$operation}Access"}
you must be kidding me. This is unreadable. Please use separate variables.Comment #63
fubhy CreditAttribution: fubhy commentedComment #64
chx CreditAttribution: chx commentedComment #66
agentrickard#63: 1696660-63.patch queued for re-testing.
Comment #68
webflo CreditAttribution: webflo commented#63: 1696660-63.patch queued for re-testing.
Comment #70
fubhy CreditAttribution: fubhy commentedComment #72
webflo CreditAttribution: webflo commented#70: 1696660-70.patch queued for re-testing.
Comment #73
webflo CreditAttribution: webflo commentedFixed the ViewsUI:access() signature.
Comment #74
fubhy CreditAttribution: fubhy commentedOkay. I have no clue what's up with the ViewsUI.php and __call() but that's not part of this issue.
Comment #75
catch@fago I think the entity_access() wrapper (with a note it's especially for access callbacks) would be good yeah. Quick re-roll with that?
Comment #76
BerdirWe've kept and renamed entity_label() to entity_page_label() for that purpose. Not sure if entity_page_access() makes sense, though.
Also, access callbacks will be replaced with those access listeners I think, that one might be able to directly call ->access() we just need to tell him how to get the entity...
Comment #77
fubhy CreditAttribution: fubhy commentedI am not sure if that makes sense really. I am currently hacking on a follow-up patch to the {slug} upcasting patch from @katbailey (#1798214: Upcast request arguments/attributes to full objects) (slightly modified though) and also on the _access one from here: #1793520: Add access control mechanism for new router system. We are really beyond the constraints of having to have stupid callbacks that we can invoke from the menu system. It's capable of much than that now. I am a big fan of the access check compiler pass introduced in that issue which would allow us to do '_entity_access' directly on the on the route instead of having a stupid callback. Can't we just skip entity_access for now (because, really... where are we going to use that once hook_menu() is really dead?!). I am going to post a follow-up patch to #1793520: Add access control mechanism for new router system (which currently adds _access and _permission) which would add _entity_access or something. But let's get this in first, it's really not blocked on that if you ask me.
Comment #78
catchSo if we don't have the separate callback, how can we get rid of this then?
Comment #79
fubhy CreditAttribution: fubhy commentedStop! It's fago time! (McHammer playing in the background).
Comment #80
catchComment #81
fago#73: 1696660-73.patch queued for re-testing.
Comment #82
fagoad #77: If we don't need the callback anymore, the better. +1
Comment #83
webchickSeems like there are quite a few things out there that would like to leverage this feature if it existed, so escalating to a "major" feature.
Comment #84
fago#73: 1696660-73.patch queued for re-testing.
Comment #85
fubhy CreditAttribution: fubhy commentedAfter discussing this with fago and catch on IRC we agreed that it would be much cleaner to extend the AccessibleInterface to support more granular $operations and move the mapping into the (not yet implemented) Field/Property access method. So with this patch the AccessibleInterface supports 'view', 'create', 'update', 'delete' instead of just 'view' and 'edit'. The mapping to 'edit' has consequently been removed.
Comment #86
fagoI like this. Changes look good. Back to RTBC given tests are green.
Comment #87
hefox CreditAttribution: hefox commentedSorry, haven't been quite following this issue, but Just going to mention that I've struggled with the limitation of node_access ops: even in d6 it's been a solid reliable way to check for access checks, but the ops is a limitation when trying to build custom access logic.
For example, premium access which has the logic that users can see a teaser version of a node but not the full content.
Also, when looking access when looking into #849602: Update 'username' theme template to use 'view label' operation., there was different types of access like there is both 'cancel' and 'delete' account with seperate logic.
so tl;dr: limited ops, limited possibilities.
Comment #88
fubhy CreditAttribution: fubhy commentedYou can always add custom $ops. The interface just defines which $ops HAVE to be supported. You can add more.
Comment #89
hefox CreditAttribution: hefox commentedOh! Cool =)
To clarify, can someone add a new op to an already defined entity (e.g. could premium add a 'view_teaser' to node entity) or would the entity be in complete control of what ops it implements (e.g. user entity have 'cancel' and 'delete' ops)?
Based on quick look at the patch, node entity is not implementing this yet?
I'm not too familair with how the entity system works (stuck in d6 land).
Comment #90
fubhy CreditAttribution: fubhy commentedHmm... To be honest: That is a pretty good question. This would not work easily as of now (you would have to replace the entity access controller for nodes in order to make that happen). However, that does not work anymore once there are multiple modules that want to add stuff. We could probably implement something along the lines of how form controllers currently work. You got a default one but you can add additional, custom form controllers.
Comment #91
fubhy CreditAttribution: fubhy commentedOh, but let's move that discussion to a follow-up... I opened #1842896: Consider adding support for custom $operations for single entity access - Let's discuss a possible implementation for that there.
Comment #92
sunI think that question rather maps to #1839516: Introduce entity operation providers
But I agree it is a very good question. Over there, I sorta arrived at the conclusion that each entity operation should probably be a plugin, so any module can add further operations or alter the existing. I almost wonder whether the entity access API shouldn't also be based on plugins, and wonder even further, whether the access API shouldn't be completely bound to entity operations instead of being a standalone thing...?
Comment #93
fubhy CreditAttribution: fubhy commentedYeah. I think I agree with that idea. Let's get this committed anyways because the implementation should still pretty much look the same. However, I am very interested in pushing this further and would like to start writing some code for that other issue asap (unless you already started with that).
Comment #94
amateescu CreditAttribution: amateescu commentedRe #92 I know I'm offtopic but I have to say this: Wow, you do wonder a lot! =))
Comment #95
xjmNifty.
I think this is potentially an excellent idea (as a followup).
Questions:
What does this mean? Is there a followup issue?
How will this interact with what we're doing over at #1658846: Add language support to node access grants and records? Presumably we'll eventually convert
Node
to implement this API?Shouldn't we be using either config or state for this rather than introducing another variable?
is_a()
is deprecated in favor ofinstanceof
: http://us1.php.net/instanceof#example-131And various doc formatting things:
Minor issue: The word "for" is repeated twice in documentation of these parameters for each method.
Verbify!
The
@see
should be at the end of the docblock. Also, it would be better to link toEntityManager
instead now.Missing the initial slash.
These should be switched to "Contains" rather than "Definition of".
I'll reroll to clean up the nitpicky things.
Comment #96
fubhy CreditAttribution: fubhy commentedWe are currently relying on a User entity there. Eventually we should move to a AccountInterface or something like that in Drupal\lib\Core so we don't reference module level code (user module -> user entity) in there.
Thanks for the detailed review.
Comment #97
xjmAlrighty, so followup issue? (= ?)
Attached fixes the rest of stuff.
Comment #99
xjmWhoops, didn't mean to change that. Reverted that one change here.
Comment #101
fubhy CreditAttribution: fubhy commented@xjm that already exists over here:
#1825332: Introduce an AccountInterface to represent the current user (@see comment #48 and before in this issue)
Comment #102
xjm*sigh* Sorry for the noise.
Comment #103
fubhy CreditAttribution: fubhy commentedThat is still a string (for instanceof to work it has to be an actual object name)
Comment #103.0
fubhy CreditAttribution: fubhy commentedUpdated issue summary. added info that #1810320: Remove EntityTranslationControllerInterface::getAccess() once have entity access is postponed on this issue.
Comment #104
xjmYeah, that's what #102 fixes.
Comment #105
fubhy CreditAttribution: fubhy commentedYeah, sorry... x-post :/
Comment #107
BerdirNote: The latest iteration of #1487760: [policy, no patch] Decide on documentation standards for namespaced items defined that the namespace is no longer necessary here.
I think this is missing a \, that's why the tests are failing.
Comment #108
xjmEr... no, I don't think so. I'm fairly sure this is probably the most important place to include the full namespace. :) It only states that the full namespace does not need to be included inline in documentation. The file docblock has its own specific rule earlier in 1354. See:
http://drupal.org/node/1354#files
http://drupal.org/coding-standards/docs#namespaces
Comment #109
xjmOh, I see more deliberation by all of 2 people was added 3 days ago. I'll respond over there.
Comment #110
xjmThanks @Berdir.
Comment #111
fubhy CreditAttribution: fubhy commentedSorry, another re-roll.
It's EntityAccessControllerInterface :/
Comment #112
webchickAwesome! This looks really good, and has sign off by all the right people.
We are over thresholds right now, but we were under over the weekend but I was at an event and unable to commit patches. So I'm cashing a raincheck. :)
Committed and pushed to 8.x. Thanks!
This will need a change notice, methinks.
Comment #113
effulgentsia CreditAttribution: effulgentsia commentedComment #114
dawehnerStarted with one on http://drupal.org/node/1862420
Comment #115
fagoThanks, I've added a pointer to the EntityAccessControllerInterface. Else, I think that's good, thus marking fixed.
Comment #116
BerdirDo we need some follow-ups here to complete this? See for example #1807776-41: Support both simple and editorial workflows for translating entities, according to that many access checks are not yet correctly implemented (which also indicates that they aren't used/covered by tests).
Maybe an issue for each entity type to convert all access checks to the new API, ensure test coverage and also make sure it's working correctly. Not sure if there already is an issue to remove at least the generic access stuff from the translation handler in favor of this, if not, that needs an issue too.
Comment #117
fagoyes, created follow-ups
#1862750: Implement entity access API for nodes
#1862752: Implement entity access API for users
#1862754: Implement entity access API for comments
#1862758: [Followup] Implement entity access API for terms and vocabularies
For the translation access I think we already have one somewhere.
Comment #118
Wim Leers#117: YAY! (See #1862750-1: Implement entity access API for nodes :))
Comment #119
larowlanSee also #1864218: Incorrect docblocks for all methods in Drupal\entity_test\EntityTestAccessController
Comment #120.0
(not verified) CreditAttribution: commentedAdded http://drupal.org/node/1825332 as a follow-up issue.