About half of the access requests I am familiar with all deal with "user with role x should be able to SEE foo content but not bar content" Is there any good reason why we don't have a view [TYPE] content permission?

Comments

Crell’s picture

Because doing so is not possible when you are dealing with lists of nodes without using the node_access table, which is a nightmare to work with.

If you can think of a non-node_access-table way to implement it, I would dearly love to see it. :-)

(I'm having dreams of query_alter here... bah.)

robertDouglass’s picture

Then why not try to solve the problem of "using the node_access table, which is a nightmare to deal with"? What is the nightmare part? That there are all sorts of realms? Performance? Should we be considering a schema that splits the table?

Damien Tournoud’s picture

Node access modules don't play that nicely with one another, and core cannot guess what the user really wants to. Having an access control by type in core would essentially prevent all other access control types to be implemented (by taxonomy, by node, etc.).

The solution to this is to install and use node_privacy_byrole, which is dedicated to that task.

robertDouglass’s picture

So if node_privacy_byrole can do it, why can't we work it into core? I don't accept that the engineering challenge is impossible.

Damien Tournoud’s picture

@robertDouglass: of course it does. But doing so it may prevent other access control modules to work properly. This should remain pluggable, IMHO.

Crell’s picture

The only way I can see to do it is with some totally funky query_alter logic based on the type, but then we'd need a a separate table from node_access to join against. That's introducing a second node access system, which makes me cringe.

mroswell’s picture

Just a note that node_privacy_byrole is listed as deprecated on its project home page.

mantyla’s picture

Some clarifications:

It is entirely possible to add "view [type] content" permissions to core (it should be named as "view any [type] content" though, to follow the naming scheme of the other permissions). It can be done the same way as all the current node permissions, without breaking the system based on node_access.

In the node_access function, the first phase is to ask the appropriate module whether a given operation should be allowed. It replies with either YES (TRUE), NO (FALSE) or UNDECIDED (NULL). For core node types, that module is the node module itself. Only if the module replies with UNDECIDED do we even check the node_access table.

Currently the node module deals with only edit, delete and create permissions, not with view. But it could manage view permissions just as it does the others.

Of course, there is still the problem with view access for node lists. But even this would be reasonably easy to do in node_db_rewrite_sql, or rather in its helper function _node_access_where_sql: we'd just have to check which node types the user may view regardless of node_access, and write some OR clauses.

Having said all that, I'm still not a fan of adding such permissions. I'm just saying there is no technical reason why they couldn't be added.

And anyway, there are good contrib modules which already do this: Node Privacy by Role is deprecated in favor of Content Access (which doesn't have a D6 version though) which is an offshoot from Nodeaccess (maintained by me).

Damien Tournoud’s picture

To clarify: at the time, when no access control modules are enabled, we use a default 'view' grant:

    // Not using any node_access modules. Add the default grant.
    db_query("INSERT INTO {node_access} VALUES (0, 0, 'all', 1, 0, 0)");

If any access control module is enabled, the default is not added, so the access control modules are responsible for using sane defaults.

So, @montyla, there are two routes here:

  • either what you are suggesting is refining the default grant, by making it type based. That's totally possible, but it means that the checkboxes will become useless if any access control module is enabled (quite confusing, right?)
  • or you are suggesting dropping the notion of a default grant and unconditionally adding grants based on node types. In that case, access control modules will not be able to restrict access to a node if access have been given in the permission page.
mantyla’s picture

or you are suggesting dropping the notion of a default grant and unconditionally adding grants based on node types. In that case, access control modules will not be able to restrict access to a node if access have been given in the permission page.

This was my point exactly. Currently we have permissions for editing and deleting based on type, and access control modules aren't able to restrict editing or deleting either if those permissions have been given.

Note that I'm not suggesting that such permissions be added, as I have my doubts whether they are such a good idea. I merely pointed out the fact that they can be added without breaking access control modules - at least, in the sense that edit and delete permissions don't break those modules.

nneuman’s picture

I marked #218038: Separate view permissions for each node type as a duplicate of this issue. However, there was a patch suggested in the other thread that might be interesting to people.

nneuman’s picture

I'd like to pick up this issue again since the current implementation of node's access seems to be counter-intuitive and inconsistent.

Just to reiterate: Currently the core node allows for explicit editing and deleting permissions based on a node type but not for explicit view permissions. If the existing permissions for (editing and deleting) are set they'll override any access control modules decisions.

To consistently handle access permissions the core module should either include view permissions or deprecate editing and deleting permissions. Since I personally favor the former, how about this:
The core node module can be patched to handle permissions just as standard permissions. This means that any access control module can make decisions and if (and only if) all of the registered node_access hooks returned NULL the core node module will apply the standard permissions. This seems easy enough to realize as the node module just needs *not* to implement the node_access hook itself but rather call the appropriate function in the basic node_access function after it invoked any other node_access modules.

EDIT: Forget the code I posted, seems I didn't quite understand how the node_access table works.

LaurentAjdnik’s picture

Subscribing

eaton’s picture

Version: 7.x-dev » 8.x-dev

Chiming in: one of the objections that's often raised is that this setup does not prevent nodes from appearing on listing pages, only on full node view pages. Naming the permission appropriately (ie, 'view full versions of [foo] content' or some such) would probably help alleviate that.

This permission makes a huge number of interesting site setups possible using nothing but core -- stuff like 'public blog with private forums' and so on. The work htat went into D7 to make simple access control hooks possible have put us very close to some very useful functionality. ;-)

Crell’s picture

eaton: If you can figure out a way around the usability problem of "you're not supposed to be able to see this node, why do you, gah, that's a security risk!", then I would support adding view permissions. But that's a more complex challenge than it appears at first blush, I think.

eaton’s picture

eaton: If you can figure out a way around the usability problem of "you're not supposed to be able to see this node, why do you, gah, that's a security risk!", then I would support adding view permissions. But that's a more complex challenge than it appears at first blush, I think.

I'm not quite clear on which aspect of the problem you're referring to -- there are a couple of different angles we can approach it from, some doable others less doable. Your recent article on palantir.net covered a lot of it, so I'm guessing you're very famliar with the different angles...

Viewing a full node can be locked down easily, and if we link it to a consistent access callback even menu links can be hidden automatically. The other issue is things appearing on listing pages if users aren't allowed to see the full nodes. At that point, I think it's a basically a semantic issue. For example, we could say, 'Core provides a way to lock down the full node view of any content type. If you want to remove things entirely from listing pages, etc, use contrib.' The problem IMO is not that we don't adhere to some arbitrary ideal permissioning behavior, it's that we need to clearly and succinctly explain what a permission controls.

Crell’s picture

Right, that's what I mean. If we simply add a "view" permission that only applies on node/$nid, not on listings/views, then I fear a lot of false security reports and people configuring their site thinking that the "view" permission does more than it actually does. That's the documentation/communication problem that I thin is more challenging than you give it credit for. :-) Of course, I may be totally wrong about that, which would be fine by me.

eaton’s picture

That's the documentation/communication problem that I thin is more challenging than you give it credit for. :-) Of course, I may be totally wrong about that, which would be fine by me.

Oh, yeah, it's nontrivial -- but if there's no way to do that effectively we probably just shouldn't have put view-level access control into core at all. Either it's usable or it's not, and if there's no way to do it, we're in trouble. Heh. Think of it as the difference between "access user profiles" and "total anonymity." We allow site builders to lock people out of user profiles, but user information still leaks through in node attribution, listing pages that display user account information, and so on.
I spent a little time thinking through the various places we actually provide listing pages, and they're broken down into a couple of categories:

  • Teaser pages for specific content types: /blog, /poll
  • Teaser pages that display numerous kinds of content: /node
  • Non-Teaser listing pages that show one content type by default, but can be expanded to include others: /forum, book hierarchies, /tracker

The first scenario is pretty easy, as the content type view permission can be paired with a listing page view permission. Voila! The second and third scenarios are trickier but is also controlled by the 'promoted to front page' flag. The key, IMO at least, is distinguishing conceptually between the teaser and the full view: 'by default, anyone can view the published teaser of a node, but special permissions are needed to view the full version of a node" might be one way to handle it. We could have node teasers not render the title into a link if the current user doesn't have permission, too.

Thoughts?

Crell’s picture

I know you're looking strictly at core, but in practice I suspect that Views accounts for upwards of 98% of all "listing" pages in the wild. That cannot be ignored when considering help text.

eaton’s picture

I know you're looking strictly at core, but in practice I suspect that Views accounts for upwards of 98% of all "listing" pages in the wild. That cannot be ignored when considering help text.

Yeah, I don't think that Views is any different than the scenarios outlined above. The only thing that can prevent things from showing up in public mixed-content lists is the grants system, and every view that's built, as best as I can tell, falls into similar categories -- content lists that can themselves be restricted, content lists that show teasers of restricted content, and so on.

Either we can articulate a decent use case for 'control access to the full node page but allow teasers,' or the access hooks shouldn't be trying to control view permissions at all -- because only the grant system is secure. Am I misunderstanding things?

eaton’s picture

Or, as a possible third case, "We can figure out how to alter the queries based on a simple permission flag that's content type specific," but I'm not sure how complex that would end up being when used in conjunction with the grants system.

Crell’s picture

Given that we would have to deal with EntityFieldQuery, not simply hook_query_alter, I don't think option 3 is viable. That leaves us with eaton's question from #20:

Can we articulate a valid and sanely describable use case to restrict access to node/$nid by type, but NOT restrict teaser lists or views?

If yes, we should do that and add view [type] permissions. If not, we should mark this issue won't fix.

eaton’s picture

Issue tags: +Needs usability review, +d8ux

Can we articulate a valid and sanely describable use case to restrict access to node/$nid by type, but NOT restrict teaser lists or views?

I think there are quite a few use cases that can be articulated, but a user building a site from scratch would have to understand the implications of the system. That sounds like a 'permission naming issue' and a UX issue for certain aspects of it. (IE, removing the title link in a teaser, and removing or altering the node links like 'Read more' if the viewer doesn't have permission to do that.)

How does something like this sound:

  • Article: View full version of content
  • Article: Create new content
  • Article: Edit own content
  • Article: Edit any content
  • Article: Delete own content
  • Article: Delete any content

Any feedback from Usability Team folks? Opinions on whether this is just asking for trouble?

larowlan’s picture

webchick’s picture

Note that my Super Simple Access II Turbo Edition sandbox project does manage to implement per-content type view permissions without touching the node_access table through hook_query_alter() and and hook_node_access(). I'm not saying that it's core-worthy (it might not even work anymore, it was coded awhile back), but this is a lot more possible now than it used to be.

And 110% agreed with this feature request as a no-brainer thing people expect out of a CMS. :)

yoroy’s picture

This is about as Snowman as it gets. This feature quickly bubbles up as a must-have in the Snowman use cases/prototypes that were written up. In the simplest terms it comes down to "visitors can see the articles, members see articles AND the forum."

Can we articulate a valid and sanely describable use case to restrict access to node/$nid by type, but NOT restrict teaser lists or views?

So that the teaser of that "WTF was foo thinking about bar!!1!" private forum thread would still show up in say, the default /node homepage for everyone? If so, that's very hmmmm then. The main use case I see in that would be something like a 'signup to read the full version' pattern, which is kinda icky too, no?

Does webchick's #26 offer a viable way around the nightmare?

DjebbZ’s picture

My thoughts :

How does this play with the idea of the Google SoC "Hierarchical permissions" mentored by chx ? If this was to be in D8, each "view mode" would inherhit from the base permission. So we could have "Article: View content" as the base, and then "Article: View full version of content" and "Article: View teaser of content" that could inherit from it, for instance.

Which leads to my second point : does this mean we should (automatically ?) create permissions for each view mode of nodes ? If I create a specific build mode (core already has Full, Teaser, RSS, Token) should I need to include a permission for it ? Or can core automatically propose a permission for it ? What about other entities ?

webchick’s picture

That sounds way too freaking fancy for what I had in mind, which is literally just a "view $content_type content" permission. We don't have a "Create $content_type in $view_mode" permission, so we would not have a "View $content_type in $view_mode" permission. Might be a useful contributed module, though.

DjebbZ’s picture

So basically core would just provide a permission for the full view, which is already a good point from a usability point of view and opens interesting perspectives as said eaton in #14. For deeper customizations, use contrib. Do I get it right ?

yoroy’s picture

Yep. Just like the title of this issue says: "Add view [TYPE] content permission". We're interested in finding the best way to make only that happen in core. Everything else is out of scope :)

geek-merlin’s picture

a strong +1 for webchicks direction!

Crell’s picture

Directly hitting hook_query_alter breaks for anything but core SQL storage. See #1186320: Get rid of the node grant system.

We need a UX answer here on whether or not "view permission for page view only" is acceptable, or else accept that toggling node-view permissions turns the grants system on. If we had a 3rd option, we'd simply eliminate the grants system in the first place. :-)

Bojhan’s picture

Yoroy notified me of this issue and we both agreed that a solution, where we expose links to and text of content people cannot actually visit is not acceptable from a UX standpoint.

I understand this is a difficult problem, but introducing this feature will create bad UX. I know even big sites struggle with this YouTube, still exposes videos in my listing that are removed or cannot view from my country (this is very annoying and jarring UX).

webchick’s picture

Yeah, I agree that's dumb.

Does core actually support non-SQL storage for nodes? I know we do for fields. But unless we support it for nodes as a whole, then this doesn't seem like a deal-breaker to me.

Crell’s picture

You can swap out the controller for nodes with a simple alter hook and stick them wherever you'd like. Examiner.com is doing so. It's not entirely clean (the node table still exists and gets data on save, but it's never read from), but for D8 it's supposed to get cleaner. So it depends what you mean by "support". :-)

yoroy’s picture

trala, ignore plz

DjebbZ’s picture

@Crell, I would love to know what this alter hook is.

Crell’s picture

hook_entity_info_alter(). chx or anyone else from the Examiner team could speak to it in more detail.

In any case, the point is that we cannot rely on nodes being in SQL 100% of the time, so we cannot rely on an SQL-based approach for node access 100% of the time.

DjebbZ’s picture

Thanks Crell.

geek-merlin’s picture

i didn't dig too deep into that, but couldn't webchick's approach be re-done with hook_entity_query_alter and then be "noslq-safe"?

eaton’s picture

It sounds like the simple answer is that Drupal core can't do any kind of access control beyond 'published' or 'unpublished' in a performance-acceptable fashion, and it is unlikely that it will be able to in the future.

Is that an accurate summary?

Bojhan’s picture

@eaton I think your summary is missing one piece of ambition.

"It sounds like the simple answer is that Drupal core can't do any kind of access control beyond 'published' or 'unpublished' in a performance-acceptable fashion, and it is likely that it will be able to in the future."

:)

webchick’s picture

Ha. :)

No, there shouldn't be any huge performance detriments with this approach (though I have not benchmarked) because it bypasses the node grants system entirely. That's why it's SUPER. :D The alter hook would need to be extended to support EFQ though for support across multiple DB systems.

johnv’s picture

IMO such a 'simple' view permission should exist. This discussion is now headin towards more sofisticated features, built upon that.

Apparently 200 D6-sites work happily with the View own permissions module.
'View the nodes in lists'/'View teasers' can be regarded as features, not bugs, and developed later. Modules like Premium content (Ah, I love the new BUEditor!) already deal with that.

If adding the permission is so complicated, why not just open up a D7-version of View_own, and let the community play with it? I think a limited contrib will draw more attention then full-fledge core-patches.

eaton’s picture

If adding the permission is so complicated, why not just open up a D7-version of View_own, and let the community play with it? I think a limited contrib will draw more attention then full-fledge core-patches.

The problem is nailed down pretty effectively in contrib space. The challenge is figuring out whether a permission like this can be effectively included in Drupal core without adding significant confusion and increasing the possibility of "accidental" exposure of private content via misconfiguration.

I think the general consensus right now is that we can't justify putting a "things show up in lists, but not full view" flag in core, in part because it would be very complex to explain and wouldn't be useful for the majority of simple use cases.

The big question -- in my opinion at least -- is the issue of hook_query_alter not working on non-sql storage backends. Theoretical support for that feature is basically preventing us from implementing Webchick's simple solution form #26 in this thread. Personally, I feel that the theoretical support for alternative storage backends is a case of hypothetical edge-case benefits degrading the experience for the 99.9999% of Drupal users. I'd be happy to say that the simple access control mechanism that ships with core simply doesn't work if you swap in a radically different storage mechanism that doesn't use SQL. There's only so much that we can do to make everyone happy, and sacrificing a basic, "elemental" CMS feature in favor of hypothetical support for nonrelational storage feels like a bad trade.

wipeout_dude’s picture

Hi all,

Let me give a use case I have which is what brought me to this feature request..

I have a $content_type that is used in a view.. Using content access I can obviously restrict "view" access on the $content_type.. When trying to set the access options in a view or in page manager its a problem.. I have the options to restrict access to those that can create, edit any, edit own, delete any, delete own or view anything published on the entire site.. What I need is to restrict it to those who can View any/own $content_type..

I have "content access" installed but this doesn't solve the problem because its not in core so there is no choice in the available "node" options to restrict by "view" permission to the $content_type.. A global access control to published content is really of no use at all..

This is a feature that I would definitely love to see in the future because Drupal is more than a basic web blog/CMS now and more of an application platform holding all sorts of data.. Granular access controls to that data is critical..

geek-merlin’s picture

working code for d7 found here: http://drupal.org/project/nodetype_access
should be easy to roll a core path from this.

for a broader picture read #1186320-10: Get rid of the node grant system

yoroy’s picture

Priority: Normal » Major

Still want this for a more better useful core UX

Bojhan’s picture

I am not sure how this qualifies as major? I love moving more usability issues to major, but I doubt this really is?

yoroy’s picture

As a feature request this would be a major step forward towards a more useful core experience. Feature requests don't have an impact on the issue tresholds, so if you have others, you can make those major too.

webchick’s picture

Status: Active » Closed (duplicate)

#139921: Add "View any %nodetype% content" and "View own %nodetype% content" permissions is older, and has a (really old) patch, so closing this one out in favour of it. Still would love to see this happen!