Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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
Comment #1
Crell CreditAttribution: Crell commentedBecause 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.)
Comment #2
robertDouglass CreditAttribution: robertDouglass commentedThen 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?
Comment #3
Damien Tournoud CreditAttribution: Damien Tournoud commentedNode 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.
Comment #4
robertDouglass CreditAttribution: robertDouglass commentedSo 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.
Comment #5
Damien Tournoud CreditAttribution: Damien Tournoud commented@robertDouglass: of course it does. But doing so it may prevent other access control modules to work properly. This should remain pluggable, IMHO.
Comment #6
Crell CreditAttribution: Crell commentedThe 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.
Comment #7
mroswell CreditAttribution: mroswell commentedJust a note that node_privacy_byrole is listed as deprecated on its project home page.
Comment #8
mantyla CreditAttribution: mantyla commentedSome 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).
Comment #9
Damien Tournoud CreditAttribution: Damien Tournoud commentedTo clarify: at the time, when no access control modules are enabled, we use a default 'view' grant:
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:
Comment #10
mantyla CreditAttribution: mantyla commentedThis 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.
Comment #11
nneuman CreditAttribution: nneuman commentedI 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.
Comment #12
nneuman CreditAttribution: nneuman commentedI'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.
Comment #13
LaurentAjdnik CreditAttribution: LaurentAjdnik commentedSubscribing
Comment #14
eaton CreditAttribution: eaton commentedChiming 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. ;-)
Comment #15
Crell CreditAttribution: Crell commentedeaton: 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.
Comment #16
eaton CreditAttribution: eaton commentedI'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.
Comment #17
Crell CreditAttribution: Crell commentedRight, 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.
Comment #18
eaton CreditAttribution: eaton commentedOh, 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:
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?
Comment #19
Crell CreditAttribution: Crell commentedI 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.
Comment #20
eaton CreditAttribution: eaton commentedYeah, 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?
Comment #21
eaton CreditAttribution: eaton commentedOr, 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.
Comment #22
Crell CreditAttribution: Crell commentedGiven 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.
Comment #23
eaton CreditAttribution: eaton commentedI 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:
Any feedback from Usability Team folks? Opinions on whether this is just asking for trouble?
Comment #25
larowlanRelated: #880940: view/revert/delete revisions per content type
Comment #26
webchickNote 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. :)
Comment #27
yoroy CreditAttribution: yoroy commentedThis 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."
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?
Comment #28
DjebbZ CreditAttribution: DjebbZ commentedMy 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 ?
Comment #29
webchickThat 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.
Comment #30
DjebbZ CreditAttribution: DjebbZ commentedSo 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 ?
Comment #31
yoroy CreditAttribution: yoroy commentedYep. 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 :)
Comment #32
geek-merlina strong +1 for webchicks direction!
Comment #33
Crell CreditAttribution: Crell commentedDirectly 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. :-)
Comment #34
Bojhan CreditAttribution: Bojhan commentedYoroy 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).
Comment #35
webchickYeah, 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.
Comment #36
Crell CreditAttribution: Crell commentedYou 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". :-)
Comment #37
yoroy CreditAttribution: yoroy commentedtrala, ignore plz
Comment #38
DjebbZ CreditAttribution: DjebbZ commented@Crell, I would love to know what this alter hook is.
Comment #39
Crell CreditAttribution: Crell commentedhook_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.
Comment #40
DjebbZ CreditAttribution: DjebbZ commentedThanks Crell.
Comment #41
geek-merlini 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"?
Comment #42
eaton CreditAttribution: eaton commentedIt 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?
Comment #43
Bojhan CreditAttribution: Bojhan commented@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."
:)
Comment #44
webchickHa. :)
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.
Comment #45
johnvIMO 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.
Comment #46
eaton CreditAttribution: eaton commentedThe 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.
Comment #47
wipeout_dude CreditAttribution: wipeout_dude commentedHi 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..
Comment #48
geek-merlinworking 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
Comment #49
yoroy CreditAttribution: yoroy commentedStill want this for a more better useful core UX
Comment #50
Bojhan CreditAttribution: Bojhan commentedI am not sure how this qualifies as major? I love moving more usability issues to major, but I doubt this really is?
Comment #51
yoroy CreditAttribution: yoroy commentedAs 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.
Comment #52
webchick#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!