Problem/Motivation

The following issues all permit access to view something using the 'access content' permission.

Is this correct? Is this correct for configuration entities? For public files?

For more context around access and REST - in some of the other issues we are adding access handlers and new permissions to deal with admin access see:

Proposed resolution

Consensus was achieved in #35:

  1. config entities that do not yet have an access control handler nor an admin_permission (RdfMapping, Tour): add an admin_permission.
  2. update all access control handlers for entity types that currently grant access without requiring any permission (View, Menu, File, DateFormat): change them to require the entity type's admin_permission or the entity-type specific "view" permission (yet to be added), so for example: AccessResult::allowedIfHasPermissions($account, ['view menu', 'administer menu'], 'OR').

There are two addenda:

  1. Per #41 + #42: we're not to add new admin permissions, but instead use the existing administer site configuration permission for config entity types that do not yet have admin permissions.
  2. Per #43: if the entity access control handler currently grants access without requiring any permission (point 2 in the original consensus), then we must at least keep the original access checking logic for the view label operation, to avoid breaking entity reference implementations.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Issue summary: View changes
vaplas’s picture

In my worldview, the files are part of the content, hence when the user does not have 'access content' this should apply to the files too. But I'm also not against the appearance of new specialized permission like "access files". And of course, I'm absolutely ready to admit my mistakes after listening to other opinions.

dawehner’s picture

The file module already uses this specific permission during the file upload:


file.ajax_progress:
  path: '/file/progress/{key}'
  defaults:
    _controller: '\Drupal\file\Controller\FileWidgetAjaxController::progress'
  requirements:
    _permission: 'access content'
~
~
Wim Leers’s picture

Priority: Normal » Major
Issue tags: +blocker

The problem is that many (most) config entity types do not specify an access control handler, nor an admin_permission.

The current status
Entity types not specifying an access control handler nor an admin_permission automatically use the default entity access control handler (\Drupal\Core\Entity\EntityAccessControlHandler). For checking access, they do this:
  protected function checkAccess(EntityInterface $entity, $operation, AccountInterface $account) {
    if ($operation == 'delete' && $entity->isNew()) {
      return AccessResult::forbidden()->addCacheableDependency($entity);
    }
    if ($admin_permission = $this->entityType->getAdminPermission()) {
      return AccessResult::allowedIfHasPermission($account, $this->entityType->getAdminPermission());
    }
    else {
      // No opinion.
      return AccessResult::neutral();
    }
  }

and:

  protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) {
    if ($admin_permission = $this->entityType->getAdminPermission()) {
      return AccessResult::allowedIfHasPermission($account, $admin_permission);
    }
    else {
      // No opinion.
      return AccessResult::neutral();
    }
  }

The latter is only for creating entities of that type. The former is for all other operations.

First necessity: admin_permission for every entity type
The above code shows that entities of a certain type can only be created if they define an admin_permission. If they don't do that, then no operations on these entities are permitted at all.

Conclusion: specifying an admin_permission is a necessity for virtually all of them.

Second necessity: a separate permission for the view operation
However, the mere existence of that admin_permission is almost never sufficient. Most if not all config entity types are safe to be viewed by many users. There's often only risk in allowing users to modify entities.

A clear example: the Vocabulary config entity. Without the ability to view these, decoupled applications cannot retrieve the label or description of a vocabulary. This is a problem.
The only solution (at least right now, and this has been the case since 8.0.0) is to give them the 'administer taxonomy' permission. But this permission then also allows them to modify and delete vocabularies (and terms!). That's far too much!
In other words: this is a clear example for the need to have a separate view permission.

The 'access content' permission.
In Drupal 8.0.0, the 'access content' permission is already being used to grant access to nodes, node_types and terms. Besides those entity access control handlers, it's used for many routes across many modules. Many of them not node-related at all. Quoting myself from #2808217-27: To be able to view Vocabulary config entities via REST, one should not have to grant the 'administer taxonomy' permission, from December 1, 2016 (>4 months ago):

I wonder if we don't want to have some permission to obey? The Block, Comment, File and System modules all use the access content permission to do this, despite not depending on the node module.

So I propose we do the same here. If we fix that at some point, we'll need to move the access content permission to the System module and provide an upgrade path anyway.

IOW: 'access content' already is the "bare minimum permission you need to access pretty much anything on a Drupal site". So… let's continue to apply that elsewhere too: non-sensitive configuration entities such as vocabulary entities would then be easily made accessible.

Wim Leers’s picture

#3:

Linking some other committed issues that made access changes under test coverage auspices.

All of these are only making minor adjustments to existing entity access control handlers. They're adding support for the view operation — the existing code only supported other operations.

Wim Leers’s picture

Also, I forgot to make one very important point in #6.

The reason this went unnoticed all this time is rather simple: Drupal 8 is not API-first, largely because our Entity/Field API has some questionable design decisions.

We're creating config entities in forms (in their submit callbacks). We're not validating those config entities. We're not checking access to those config entities. We're only checking access to the form. Hence for config entities, we are bypassing our validation and access APIs.
This works … but as soon as you want to be able to perform those same tasks via a REST API (or GraphQL or JSON API or something else), you're then forced to finally define the hitherto undefined access control. (And for validation we have #2300677: Create/Update/Delete (POST/PATCH/DELETE) ConfigEntity via REST.)

Wim Leers’s picture

I bumped this to "major" in #6 because this is blocking 7 issues that are effectively RTBC, but they're all implementing a change that we need to agree on in this plan issue. Alternatively, this plan issue must specify a different approach.

Wim Leers’s picture

Status: Active » Needs review

This is actually the appropriate state.

dawehner’s picture

First necessity: admin_permission for every entity type
The above code shows that entities of a certain type can only be created if they define an admin_permission. If they don't do that, then no operations on these entities are permitted at all.
Conclusion: specifying an admin_permission is a necessity for virtually all of them.

Well, the admin permission seems to be more of a shortcut ... it feels better to define a custom access control handler on the longrun, given that people want specific permissions anyway.

Wim Leers’s picture

#11: Agreed that you want fine-grained, well-thought-through access control handlers for every entity type … but an admin_permission is still useful for the coarse "be able to do anything with this entity type", which is also a valid need.

dawehner’s picture

That's fair, well to be honest I totally agree with your for core for now. The problem is once we add this admin permission we need to carry it around potentially, so let's better think about it before we add it.

Wim Leers’s picture

The problem is once we add this admin permission we need to carry it around potentially,

What do you mean? It's up to every entity type individually to specify an admin_permission. So the impact is always limited to that one entity type.

dawehner’s picture

What do you mean? It's up to every entity type individually to specify an admin_permission. So the impact is always limited to that one entity type.

Right. Let's assume though we adapt the entity type in the future to have more granular permissions. In that case we still need to provide support admin_permission.

Wim Leers’s picture

True. But for most of these entity types, there already is a module-level administrative permission!

I agree though we need to consider that long-term cost. Do you see an alternative approach to avoid that?

dawehner’s picture

Now that I think about it, config entities, unlike content entities, are created/changed by the same group of people (site builders).
These people use an admin role anyway, from my point of view, so given that, I think it is actually fine to add administrative permissions to every of those entity types.

What are your thoughts about that?

Wim Leers’s picture

My thoughts exactly! :)

Wim Leers’s picture

Issue tags: +Baltimore2017

Just discussed this with @catch at DrupalCon Baltimore 2017.

Outcome:

  1. He agrees that access content is the sensible permission to use for this; because it is already being used outside the node module anyway.
  2. He is concerned that we are allowing people to view view config entities with just the access content permission, but he agrees that doing so is a step forward compared to today (because today we grant permission always, no permissions needed). So he wants to see a major follow-up tagged Security improvement to review that.

I think that the next step is for Alex Pott to agree or disagree. Then:

  • if he agrees, he can RTBC it. Then I can unblock all the blocked issues and get them moving forward again.
  • if he disagrees, we need further discussion.
alexpott’s picture

I'm okay we the decision - I have a mental dissonance with access content granting access to view configuration - but having this controlled by a permission definitely makes sense. I'm right in thinking that you can configure more granular permissions on the rest.resource config entity if you want?

Wim Leers’s picture

but having this controlled by a permission definitely makes sense

Yep, it's not controlled by any permission right now.

I'm right in thinking that you can configure more granular permissions on the rest.resource config entity if you want?

No; you only get the extra permissions if you enable the BC layer that #2664780: Remove REST's resource- and verb-specific permissions for EntityResource, but provide BC and document why it's necessary for other resources left in place. What do you mean exactly when you say more granular permissions? I could see it be valuable to require the administer views to just view a view (heh), but most others seem 100% harmless to expose to just about anybody: DateFormat, RdfMapping, Menu, and so on.
So, can you please clarify?

dawehner’s picture

For me viewing a config entity as a thing is tricky outside of REST.

alexpott’s picture

@dawehner yep that's why this is such an odd issue. Thinking about this some more I really think that viewing raw configuration should be a separate permission from access content. Taking the view example you might not want to expose your business configuration to anyone that can access content on your site. Whilst there might no harm there certainly is disclosure and you might view your view as intellectual property that you only want certain users to be be able to access.

Wim Leers’s picture

But that's not true for Vocabulary config entities, which you need to be able to view to be able to know the name of the vocabulary. This is the #1 use case.

Berdir’s picture

Not sure if this was mentioned already or not..

The reason node type is controlled by access content is that this used to be the only way to be able to see the node type label on /admin/content if users didn't have administer node types.

It's now possible to do this more fine-grained with the view label operation. See #2692091: Use the new 'view label' entity access check in the entity reference label formatter (specifically comment #38), #2471154: Anonymous user label can't be viewed and auth user labels are only accessible with 'access user profiles' permission and #2561331: When 'administer content types' permission is disabled, content type field is hidden in content manager.

So we could change node type now to only allow label access with access content, not sure if that's an API/something change or not?

dawehner’s picture

Taking the view example you might not want to expose your business configuration to anyone that can access content on your site.

There is a bit of a strong point here ... especially you don't want to expose some configuration entities suddenly after an update.

amateescu’s picture

FWIW, I agree with #23, #25 and #26, so I think the answer to the current issue title is pretty much 'nope' :)

Wim Leers’s picture

#27 but that means we make a very strong BC-incompatible change: e.g. the View config entity currently requires zero permissions. Requiring access content is a non-BC breaking solution for that.

And again:

But that's not true for Vocabulary config entities, which you need to be able to view to be able to know the name of the vocabulary. This is the #1 use case.

This then needs a different answer.


To be clear: I'm perfectly happy to accept "require the admin_permission for any operation on any config entity" to be the default answer. But for e.g. reading a Vocabulary config entity, we need something else, because for that config entity type, viewing the config entity is a hard requirement: we need the name, the description and the hierarchy for building decoupled apps. Viewing those is harmless, but having all CRUD permissions (which is what granting its admin_permission would result in) is a no-go.

Therefore: I'm fine with not going with access content as the general rule. So that e.g. view config entities will require the corresponding admin permission. But then what about harmless things that are important for the client side, such as Vocabulary, DateFormat and Menu?

dawehner’s picture

Requiring access content is a non-BC breaking solution for that.

I'd argue its breaking some BC in regards to access right now. Why can't we have a 'view config' permission instead?

Wim Leers’s picture

Requiring access content is a non-BC breaking solution for that.

I'd argue its breaking some BC in regards to access right now. Why can't we have a 'view config' permission instead?

I disagree, because how many sites in practice do not have the access content permission granted to every role? Less than one in a thousand, I bet — probably even less than 1 in 10K.

Adding a new view config permission would surely be more disruptive? Also, a single view config permission for all config entities would be quite dangerous, because there's a huge difference in security sensitivity between being able to view a Vocabulary config entity vs a View or RestResourceConfig config entity?

That being said: I'd personally like that for sure.


But more importantly, you did not reply to my proposal below the <hr> in #28. I'm wondering if you have thoughts about that?

dawehner’s picture

I disagree, because how many sites in practice do not have the access content permission granted to every role? Less than one in a thousand, I bet — probably even less than 1 in 10K.

Maybe I understood wrong what you've been saying. I thought you want to actually use 'access content' for all configuration entities.

I think we have a conflict of
a) Denying people access, when they used to have access
b) Allowing people to gain access, when they used to not have access

I really lean towards a), while your arguments seems to move towards b) more.

Also, a single view config permission for all config entities would be quite dangerous, because there's a huge difference in security sensitivity between being able to view a Vocabulary config entity vs a View or RestResourceConfig config entity?

At least for content entities most of them have a specific permission right? Given that I think we should have a look how a better build system looks like. In this world I would have expected permissions to view specific entities. We can ship in standard install with permissions for the anonymous use to access vocabulary/menu/display format, but maybe not the other entity types?

But more importantly, you did not reply to my proposal below the


in #28.

I hope its okay what I write in the following paragraph ...

Total unrelated, you seem to add more and more visual glutter to structure your comments. Maybe its just me, but it makes it actually harder to actually be able to read them in a reasonable amount of time. If you read a long text, visual markers are kinda like constant distractions for your eye.

Wim Leers’s picture

I think we have a conflict of
a) Denying people access, when they used to have access
b) Allowing people to gain access, when they used to not have access

I really lean towards a), while your arguments seems to move towards b) more.

I'm fine with that actually (because more secure), but I was concerned about the BC break that that implies. I agree that the BC break is mostly theoretical though. So if you're fine with that, then I'm too.

We can ship in standard install with permissions for the anonymous use to access vocabulary/menu/display format, but maybe not the other entity types?

+1

So then, to bring this all together in a concrete proposal again:

  1. config entities that do not yet have an access control handler nor an admin_permission (RdfMapping, Tour): add an admin_permission.
  2. update all access control handlers for entity types that currently grant access without requiring any permission (View, Menu, File, DateFormat): change them to require the entity type's admin_permission or the entity-type specific "view" permission (yet to be added), so for example: AccessResult::allowedIfHasPermissions($account, ['view menu', 'administer menu'], 'OR').

P.S.: I use <hr>s to separate either thoughts, or replies to different comments. I get super confused when people reply to many things, or have many distinct thoughts, and put it all in one super long comment, with some paragraphs belong together, and some not. I guess we process/read information differently :) No offense taken in any case!

dawehner’s picture

+1 to your summary. I think when we introduce it, we should certainly back it up by having REST test coverage for them. This reduces any potential risk around it.

I guess we process/read information differently :) No offense taken in any case!

Sure, I just wanted to write what I think :)

alexpott’s picture

#32 sounds like a great - we can alway relax the permissions later if we choose to for specific config entities.

Wim Leers’s picture

Status: Needs review » Fixed

HURRAY! We have consensus!

So the consensus is:

  1. config entities that do not yet have an access control handler nor an admin_permission (RdfMapping, Tour): add an admin_permission.
  2. update all access control handlers for entity types that currently grant access without requiring any permission (View, Menu, File, DateFormat): change them to require the entity type's admin_permission or the entity-type specific "view" permission (yet to be added), so for example: AccessResult::allowedIfHasPermissions($account, ['view menu', 'administer menu'], 'OR').

Now updating the 7 issues in the IS to comply with this consensus.

Wim Leers’s picture

Issue summary: View changes

Updated IS to reflect current state.

vaplas’s picture

Great news! Thanks to everyone for promoting this problem, and special thanks to @Wim Leers!

Berdir’s picture

Hm, sorry for being late to the party, but I'm not quite sure that we've discussed everything here.

So, first a question:

What exactly is the purpose/use case of allowing REST access to config entities? Who is going to need it, to do what?

The conclusion here is that we basically require administrative permissions for anything on many/most config entities, even viewing them. But as far as I see, that's a problem for a number of things:

* I looked quickly at the Menu patch, it actually changes the access control handler, what if code relied on that? Conceptually, I'd say that the menu block should check view access for a menu, because then you could actually *do* something with it, like denying access to a whole menu für some users. another access example that affects many config entity is when combined with entity references, because those check view access. As mentioned in other issues, that's for example why NodeType and Vocabulary config entities allow view access with access content. We now have the view label access operation, so we could say that we at least still allow access to that for those types, but again, I'm not sure if that is an behavior change. what if people have use cases that involve referencing a menu on a node, for example to show it inside the node. then this will break with those patches as users will no longer be able to see the menu labels.

* I assume a common case for REST access for config entities is decoupled applications, so that they're capable of displaying the node type label for example, or do some rendering based on its settings. Or format a date, or show tours, or add rdf mappings. But that's impossible then unless you have an admin user, so that's obviously not a solution. But at the same time, as pointed out by myself as well, allowing full access to those config entities is also not an option, with third party settings and so, who knows how much information is in there. But how are we going to solve this use case then? should we offer certain API's as services instead of just exposing data? Is this something that sites need to do themself, with exactly the data they need? Should we for example expand content entity serialization to include things like labels of referenced (config) entities?

Yes, these questions go beyond the actual question of this issue, but I'm wondering if we solved the issue without actually solving the problem behind it?

Wim Leers’s picture

I agree we should have fine-grained "view" access permissions rather than just requiring "admin" permissions. But requiring "admin" permissions is just a first step. We will be able to evolve this in the future: also allow "view" access if the user has some other permission, or if they can access some other thing. What we need to fix today is: a) unregulated access (granting access to all without any condition), b) not having access control at all (because no access control handler and no admin permission).

catch’s picture

Status: Fixed » Active

Sorry I'm re-opening this partly due to Berdir's comment, partly due to #2843768-21: EntityResource: Provide comprehensive test coverage for Tour entity.

If we add an administer permission for every config entity type that doesn't have a user interface, then that's several permissions in the UI that are a no-op unless REST is enabled. To move forward quickly without any UI changes, could we use 'administer site configuration' to start with, then add more permissions later on when we're more sure we need them?

Wim Leers’s picture

Issue summary: View changes

#41: hah, I proposed exactly that in #2843768-22: EntityResource: Provide comprehensive test coverage for Tour entity! So let's move forward with that. IS updated.

Wim Leers’s picture

Issue summary: View changes

At #2843772-20: EntityResource: Provide comprehensive test coverage for DateFormat entity, @Berdir raised this:

Some things are checking view access, for example entity reference fields as mentioned in the other issue. I'm not sure if there is a site out there with a date format entity reference, possibly not, but there are a number of config entities with perfectly valid use cases or where we're even doing it by default, like NodeType.

As I kind of suggested, if we do this, we should consider to at least keep the previous access for the view label operation.

Also, what's interesting is that viewing doesn't really mean the same for REST as it does for other kinds of viewing. It's perfectly valid to "view" a block in a traditional way, meaning, passing it through BlockViewBuilder. but viewing for rest is printing out its raw configuration, which is a very different thing.

For content entities, we have the field access as an additional layer of security. I'm pretty sure that many sites are accidently exposing a lot of internal data/fields through REST because usually those kind of fields are hidden by simply not printing them in the node template/view display configuration. But there we can at least claim that we *have* an API to solve that problem :)

(emphasis mine)

To which I replied in #2843772-22: EntityResource: Provide comprehensive test coverage for DateFormat entity:

As I kind of suggested, if we do this, we should consider to at least keep the previous access for the view label operation.

So you're saying that if this existed:

if ($operation === 'view') {
  return AccessResult::allowed();
}

then we must at least do this:

if ($operation === 'view label') {
  return AccessResult::allowed();
}

?

I think this makes sense. I see that Berdir mentioned this before in #39, but we overlooked/forgot that. Updated conclusion in IS with another addendum.

alexpott’s picture

Status: Active » Needs review

I like the updated direction in #43.

Wim Leers’s picture

Issue summary: View changes

Great :) Hopefully Berdir, catch and dawehner also agree.

dawehner’s picture

I agree with the new direction in #43 as well.

Berdir’s picture

administer site permission: Works for me to start with that.

view label operation: I was the one who suggested that, so I can't really disagree there :) Your patch is not enough yet however, see https://www.drupal.org/node/2661092, you need to opt-in for that operation. Should we add test coverage for that? To counter your next argument, yes it is a REST test patch, but what you are changing is not rest specific :)

I don't think those two things answer all of my questions, though? If an admin permission is just a first step, fine, but we should probably at least have some kind of plan/issue to discuss what the actual use cases are for reading this data (e.g. decoupled sites) and figure out how to get there? I kind of like my idea above to enrich the content entity serializers with labels and things that might be useful for decoupled sites. Possibly based on some kind of context.

Wim Leers’s picture

RE: view label operation needing protected $viewLabelOperation = TRUE; in the entity access control handler. /me throws up hands in despair.

RE: Should we add test coverage for that? To counter your next argument, yes it is a REST test patch, but what you are changing is not rest specific :) OMG… you're right of course, except the maddening and frustrating truth is that the answer should be expand test coverage, but of course, zero test coverage exists for most of these access control handlers. Definitely for DateFormatAccessControlHandler.

I won't deny it: this is extremely frustrating. This slows down progress in the API-First Initiative enormously. The API-First Initiative has to fix massive oversight after massive oversight in the Entity/Field API. In fact, the majority of the work has been outside the API-First Initiative's modules for a very long time. The majority of the work is not in the rest, serialization and hal modules, but in the entity system, request processing system, routing system components, plus in every module that provides an entity type.
Of course, making an API-First Drupal a reality, those are exactly the things we must mature/stabilize/add test coverage for. But it's such a monotonous, mind-numbing, hard-to-find-reviewers-for, thankless, slow, frustrating, endless, boring job that I wish oh so very much that we didn't have quite as many oversights.

See #2843772-25: EntityResource: Provide comprehensive test coverage for DateFormat entity for what I then guess must be the first of many first rounds of add comprehensive entity access control handler test coverage


I don't think those two things answer all of my questions, though? If an admin permission is just a first step, fine, but we should probably at least have some kind of plan/issue to discuss what the actual use cases are for reading this data (e.g. decoupled sites) and figure out how to get there?

The actual use case is exactly what you say: making it possible at all to build decoupled sites that interact with configuration entities.
Yes, we'll need #2300677: Create/Update/Delete (POST/PATCH/DELETE) ConfigEntity via REST for that too. But ensuring access control behaves as expected, and having the ability to prevent viewing certain configuration entities must happen first.
Without having actual CRUD capabilities for config entities, nobody is ever going to build ambitious decoupled UIs that talk to Drupal sites. Because it is literally impossible. There will still be bugs after all this, but right now it's not remotely possible, and it requires a titanic effort to get it to a "mostly-working" state. Which is exactly what I've been doing for the past year with a few other people. And as this issue demonstrates, there are still lots of basics completely wrong/missing.

I kind of like my idea above to enrich the content entity serializers with labels and things that might be useful for decoupled sites. Possibly based on some kind of context.

?!? I have no idea what this is referring to, and it sounds very vague. So I tried to find what comment of yours you were referring to, and it seems to be this part of #39:

* I assume a common case for REST access for config entities is decoupled applications, so that they're capable of displaying the node type label for example, or do some rendering based on its settings. Or format a date, or show tours, or add rdf mappings. But that's impossible then unless you have an admin user, so that's obviously not a solution. But at the same time, as pointed out by myself as well, allowing full access to those config entities is also not an option, with third party settings and so, who knows how much information is in there. But how are we going to solve this use case then? should we offer certain API's as services instead of just exposing data? Is this something that sites need to do themself, with exactly the data they need? Should we for example expand content entity serialization to include things like labels of referenced (config) entities?

Well that's the thing, the consensus in this issue is to provide non-admin permissions to view these config entities, and admin permissions to modify them. If you're allowed to view a config entity, then yes, you're also allowed to view third-party settings. Surely there's no harm in that? If some third party settings require their own access control, then they shouldn't be third party settings, then they should have their own config entities (so they can have their own access control)! In the third party settings you could then still refer to these associated config entities by config entity ID, which is once again harmless.
If that's not really what you were getting at, and you were actually implying that it's impossible to strictly/sensibly validate third party settings, because there exists no proper validation system for config entities in the first place, then you're right, that's another massive, painful oversight that we need to fix now… For which I again point you to #2300677: Create/Update/Delete (POST/PATCH/DELETE) ConfigEntity via REST.

P.S.: none of this is directed at you, Berdir, but at the overall lamentable state of things from an API-First point of view. It's unfortunately a titanic effort, if not a sisyphean effort. Once software has shipped, and BC must be maintained, it's extremely difficult to make something that's API-second actually API-first without breaking things. It's absolutely understandable why we're in the state we're in. But we need to get much better at determining/communicating what is actual supported API and what is not, because this is very much death by a thousand cuts: unmaintainability by a thousand oversights.

P.P.S.: part of the reason for writing this high-level rant is that this issue finally got to consensus after a month, but as of #47 it seems the scope of this is ever-expanding. And this after half a dozen issues were already blocked on feedback for weeks, which is how this issue was opened in the first place. And the reason those half dozen issues (out of a total of >30, see #2824572: [PP-11] Write EntityResourceTestBase subclasses for every other entity type.) exist in the first place, is that neither the REST module nor the entity system had the necessary test coverage in place to project sufficient trust, because anybody using D8 REST a year ago was running into weird problem after weird problem. At this fixing pace, it will take at least another year.

dawehner’s picture

The actual use case is exactly what you say: making it possible at all to build decoupled sites that interact with configuration entities.

I think we should even think about it more broadly. We want to be API first, and not API second. This sadly involves all the pain points @Wim Leers mentions in his previous post. I think the only way to resolve that is to ensure that at least the other side of the world is API first for every new thing, rather than making it an afterthought (random example: how do we post media entities).

Without having actual CRUD capabilities for config entities, nobody is ever going to build ambitious decoupled UIs that talk to Drupal sites. Because it is literally impossible.

Well, to be fair its not impossible as we speak. JSONAPI exposes config entities in its full variety already, and well, you can always easily swap out things, when you actually need it. You will never access every config entity, but rather you build UIs specific per bit, which means that you will have code for each configuration already.

I kind of like my idea above to enrich the content entity serializers with labels and things that might be useful for decoupled sites. Possibly based on some kind of context.

As far as I understand this statement we are talking about exposing more information to the normalizers than the pure stored data. label() is one of those examples, toUrl() might be another one. They contain information which are derived from the stored values, but still are distinct between different entity types.

Berdir’s picture

Yes, what I mean for example is that reference fields would include the label of the referenced entity, so you get things like term labels but also author name, node type labels, vocabulary labels and so on diretly as part of the main entity you're requesting, e.g. when you specifiy ?_reference_labels or something.

More on the other things later.

Wim Leers’s picture

#49:

This sadly involves all the pain points @Wim Leers mentions in his previous post.

Thanks for acknowledging the pain.

I think the only way to resolve that is to ensure that at least the other side of the world is API first for every new thing, rather than making it an afterthought (random example: how do we post media entities).

YES YES YES to this. Not to mention normalizers etc.

Adding new things to core == integrating it with all the things. Not providing full normalization support means not providing full REST/JSON API/… support. Which means that this increases the burden on the API-First Initiative contributors.

JSONAPI exposes config entities in its full variety already

… but because of that, huge security caveats are attached to that.


#49 RE: label() and toUrl(): it still isn't very clear to me I'm afraid. Are you suggesting "view modes" for normalizations? i.e. a "label normalization" for an entity, and a "URL normalization" for an entity?

#50: Oh, I think you're saying "ability to normalize not just references by ID, but also already include some fields from the references, such as label and URL". I think I agree with that :) But I am also certain that's vastly out of scope here. It's only tangentially related in that "normalize labels" should use the view label access operation.
But let's not forget that this is completely useless for many use cases. For example: vocabularies. You could argue you only really need the label. Well, that's not true: you also need the hierarchy and description for most decoupled applications!
Finally: JSON API already kinda solves this for the most part, thanks to ?include and ?fields see http://jsonapi.org/format/#fetching-sparse-fieldsets + https://www.drupal.org/docs/8/modules/json-api/fetching-resources-get.

dawehner’s picture

Adding new things to core == integrating it with all the things. Not providing full normalization support means not providing full REST/JSON API/… support. Which means that this increases the burden on the API-First Initiative contributors.

Does that mean we should propose a new core gateway, which is called MAPI first??

#49 RE: label() and toUrl(): it still isn't very clear to me I'm afraid. Are you suggesting "view modes" for normalizations? i.e. a "label normalization" for an entity, and a "URL normalization" for an entity?

Well, no, but at the moment there is for example no quay to get the label of an entity via rest.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dagmar’s picture

I created #2892821: Move 'access content' permission from node module to system module few weeks ago. I would like to see some input there, seems related to this topic.