Content Access disables other node access modules [#226090]

CA's "optimization" of node access records inserts a

NID, 'all', 0, 1, 0, 0

record for every node that is not specifically controlled by CA. This is exactly how it's NOT supposed to work.

Drupal allows access to be granted, but not revoked. Drupal core creates such a record when a node is not controlled by any node access module, and it removes that record if any node access module provides at least one grant. Content Access must not put that record back!!! One of the modules that is overridden in this way is Forum Access.

As a matter of principle, node access modules must create grant records in their own realm(s) — no contrib module must ever create 'all' records!

Comment	File	Size	Author
#4	CA_remove_view_optimize_2.patch	961 bytes	amitaibu
#1	CA_remove_view_optimize_1.patch	939 bytes	amitaibu

Comments

Comment #1

amitaibu

Hebrew

Israel

commented 16 March 2008 at 14:37

Version:	5.x-1.3	» 5.x-1.x-dev
Status:	Active	» Needs review

Status	File	Size
new	CA_remove_view_optimize_1.patch	939 bytes

This patch basically removes the 'view' grant optimization.

Comment #2

salvis

he/his

commented 16 March 2008 at 15:37

Thank you for the patch — let's hope fago commits it soon...

Comment #3

fago

German

Vienna

commented 17 March 2008 at 11:39

Status:

Needs review

» Closed (works as designed)

hm, I'm not sure if you are right.
CA only does this optimization, if you have view access enabled for both anonymous and authenticated users. As you told, the drupal node access system allows only to grant access, not to deny. So if you users grant access to everyone, then this optimization is correct.

So if one doesn't want everyone having success, don't configure CA that way ;)
Feel free to reopen, if you think I'm wrong.

Comment #4

amitaibu

Hebrew

Israel

commented 18 March 2008 at 10:55

Status	File	Size
new	CA_remove_view_optimize_2.patch	961 bytes

Fago,
I've checked in OG module. When a node is public it writes it under og module:

test og_public 0 1 0 0 All users may view this node.

This patch keeps the optimization, but changes the realm and the priority according to CA.

Comment #5

salvis

he/his

commented 20 March 2008 at 02:15

Status:

Closed (works as designed)

» Active

@fago: Are you saying that if module A grants View to anonymous and module B to authenticated, then you replace that with grant View by 'all'?

Or if FA grants View to anonymous and authenticated, then you replace it with grant View by 'all'? How could the FA user subsequently remove View from anonymous, for example? You've broken FA!

I maintain that no module must ever touch another module's grants (unless the two cooperate explicitly), and no contrib module must ever touch the 'all' realm.

Comment #6

fago

German

Vienna

commented 20 March 2008 at 09:57

Status:

Active

» Closed (works as designed)

salvis, the optimizations are only for content access. It doesn't touch any other module. When someone configures access to anonymous + authenticated users, he grants access to everyone, so the all grant is inserted. Imo this assumption is correct and so the optimisation.

amitaibu, your patch won'T work as this realm isn't known. Anyway, I don't think that this would change something in contrast to the "all" realm. So let's stay with that.

Comment #7

salvis

he/his

commented 22 March 2008 at 16:39

Status:

Closed (works as designed)

» Active

@fago: please see
#215563: Unpriviliged users can access thread and
#226000: Private forum thread are visible by all in "recent post" view (tracker view)

If "the optimizations are only for content access," then write 'content_access' grants, but not 'all' grants! Then the user has a chance to figure out why his content has suddenly become visible to everyone.

Or call the realm 'content_access_all' or whatever you like, but do put the name of your module into the realm name, and don't hijack core's 'all' realm for your own purposes!

An 'all' record must only be present, when no other node_access records exist for the node.

Comment #8

fago

German

Vienna

commented 23 March 2008 at 15:55

Status:

Active

» Closed (works as designed)

hm, actually we can discuss whether hijacking 'all' or not is a good idea or not (if you want to please open a new issue). But this doesn't matter for this issue. If I do a 'content_access_all' grant things would behave exactly the same way.

As I said before, when one module grants access, access is granted. This is the way drupals node access API works and content access can't and won't change that.