DA and TAC with multiple node access patch?

This issue is looking for testers.

Update: this original note is false, see comment #3.
[Note that TAC has its own patch to node_access, I believe. So tests for both cases should be run. ]

--far as i know TAC doesn't patch the Drupal core, and i was using it for 7 months or so.

b
[edit. i forgot to say: Happy New Year to you, agentrickard]

ben soo is correct. I confused TAC with another module that used the nodeapi 'access' patch approach.

Since TAC uses the standard node_access system, there should not be a conflict. So this is a good test of the mutliple_node_access patch, which was tested against OG.

Testing.

Bad things happen to DA when node_access_rebuild() is run. It seems that the issue is related to the fact that node_access_rebuild rewrites all existing node access rules. Among other things, the domain_all grant is removed entirely.

With the Domain Content module, you can restore these, but it's still an issue.

The short answer to the original question is "No. Not unless you never delete a term or disable TAC after you install the two modules."

This is, IMO, a flaw in the node_access system. node_access_rebuild() _by design_ erases the {node_access} table, which means that the node_load() that executes later in the function can no longer read the existing Domain Access rules, which were stored -- and deleted -- from the {node_access} table.

http://api.drupal.org/api/function/node_access_rebuild/5

TAC also calls node_access_rebuild() when a taxonomy term is deleted, which will also destroy DA records.

function taxonomy_access_taxonomy($op, $type, $array = NULL) {
// ....
      case 'delete': // delete everything from term_access and node_access
        db_query('DELETE FROM {term_access} WHERE tid = %d',$array['tid']);
        node_access_rebuild();
        break;
}

See http://groups.drupal.org/node/7956 for discussion.

If I store all the DA node_access rules in a {domain_access} table, then this integration seems to work fine.

Calling this, officially, a bug. I have new code ready to implement this.

It means I must release an rc3 now, which must be tested all over again, since I made a fairly major change to the module architecture.

Upgrading to critical.

The fixes have been committed to HEAD.

Active in RC3. DA and TAC should play nice now. The multiple_node_access patch is needed only in the following cases:

-- If you want DA to check its grants in addition to TACs. (Drupal default is either DA or TAC).
-- If you want to use the "domain editor" functions of DA.

i've just been reading through your entries concerning this issue. That's pretty amazing detecting and debugging, agentrickard, not to mention an admirable tenacity.

So i guess that's what i ran into, this node_access_rebuild() thing? It seems to've caused repercussions beyond the DA module which i can't pin down or fix, and so am moving the site to a fresh install which will have DA RC3.

Going to see for sure whether it'll coexist with SomebodySysop's OG User Role.

Finishing up the prep work; almost ready to start importing the existing content tables into it. Just beginnig to figure out which DB tables have bits of our content in them.

--This is a feckless question and i apologize beforehand: do you have recommendations on which tables i should specifically avoid importing, in order to be sure of leaving behind any problems that might've been caused by this issue?

thanks,
b

I don't understand the question. From what system are you importing data tables? A previous version of Drupal?

--From the same D5.6 version into a fresh installation. The old installation is giving me theme problems whereby the 4 or so different themes i use in various parts of the site all revert to the system theme once i switch on OG User Roles. Even the administration theme goes away.

The new installation, so far, with a similar but smaller set of 3rd party modules, isn't giving me this problem. Everything co-existing nicely.

Well, I can't really answer the table import question. Some thoughts:

- DO NOT IMPORT

- cache_*
- variables

- DO IMPORT

- node*
- term_node
- term*
- vocabulary*
- sequences

You might be better off with a node import migration.

As for og user roles, check to see if it overwrites the $custom_theme variable anywhere -- usually this happens in hook_menu().

We should leave this issue and start new ones if you run into issues with OGUR.

Not exactly. The patch support AND or OR logic for all node access modules. However, only DA implements it.

So, if you have 4 node access modules, your queries can be either:

NA or OG or TAC and DA
-or-
NA or OG or TAC or DA

Where the second is the current and default behavior.

That was great and wonderful advice, that i shouldn't import the "variable" table.

i've put our site back together under a new install and have been proceeding with its construction. RC4 is generally nice on the site, except Domain Theme doesn't play nice with Taxonomy Theme: they both make use of $custom_theme .

i'll file a separate issue for that. i've already done so in the issue queue of Taxonomy Theme.

b

Hi. There continues to be a conflict in using DA and TAC together, even with the multiple node access patch.

Steps to reproduce:
- Drupal 6.19 with DA's multiple node access patch, taxonomy_access 6.x-1.2, DA 6.x-2.5 (only the "Domain Access" module enabled from the DA package)
- admin/build/domain/settings -> Advanced settings -> Check Domain Access in addition to other modules (AND)
- Create a "Privacy Settings" vocabulary, applicable to "Page" types, and add 1 term in it ("Authenticated Only")
- admin/user/taxonomy_access, edit "anonymous", add a record for "Authenticated Only", and check the "D" radio buttons (i.e., deny all access to anonymous users for content that has the "Authenticated Only" taxonomy term)
- Create two "Page" nodes. Give one the "Authenticated Only" term. Do not give the other one that term.
- Go to admin/content/node-settings and "Rebuild permissions". All is correct. The node without the "Authenticated Only" term is viewable by anon. The node with that term is not.
- Now, edit the node without the term and save it. Now, anon cannot view it.
- "Rebuild Permissions", and all is correct again (anon can view the prior saved node).

Expected behavior: After saving a node, permissions should be correct. It's wrong to require a "Rebuild permissions" after every node save. Content editors who save nodes may not have permission to admin/content/node-settings.

Info that may be helpful:
- When DA is disabled, so TAC is the only NA module, when you save a node that does not have a taxonomy term, there are no node_access records written for that node for the 'term_access' realm. Instead, there's just 1 record written for the node, and it has the 'all' realm. However, when you click "Rebuild permissions", the "all" record is replaced with 'term_access' records.
- When DA is enabled, when you save a node that does not have a taxonomy term, node_access records get created for the 'domain_site' and 'domain_id' realms, but there are no records for that node for the 'term_access' realm. However, when you click "Rebuild permissions", the 'term_access' realm records get added.

Is this a bug with TAC, with DA, or with the multiple node access patch?

Cross-linking #897764: Conflict with Domain Access module.

Bug with core, IMO. The issue is that TAC expects the 'all' grant to be written if it asserts no rules, but it doesn't because there is another node access module enabled.

This is not really fixable directly in either module.

The best solution for this in D6 is to use hook_domainrecords() to ensure that the proper permissions are written. In this case, insert an a TAC grant when taxonomy terms are empty, or an 'all' grant.

See the logic in http://api.drupal.org/api/function/node_access_acquire_grants/6 for why this happens.

In theory, we might be able to adjust the patch to allow for this, but the patch is really a dead-end, so I have little interest.

I use TAC successfully with DA and Domain Access Advanced. The disadvantage, of course, is that it makes DA use hook_db_rewrite_sql() rather than the node access system, but it does work.

The problem with #22 is that this modification in TAC would bludgeon any other node access modules installed, unless I'm mistaken. (E.g. TAC + CA + DA, and I know there are people who do stuff like this because I get issues about it all the time).

Edit: I was talking to hefox the other day about this on IRC. Hefox had some alternate solution for making DA work with other node access modules, but I don't remember atm what it was. I'll see if I can find my notes about it.

Possibly http://drupal.org/project/module_grants

Confirming that http://drupal.org/project/module_grants solves the conflict described in #20, because of the lovely Interpret absence of access grants as a "don't care", rather than a "deny access". setting on admin/settings/module_grants (on by default, and what is desired for this use-case). This is just from initial testing. There may be other issues that pop up, but I'll leave it to whoever discovers those to re-open this when they do.

Follow up to #20-26. Using Module Grants did, indeed, solve the problem in #20. However, it now seems that the Domain Access permissions are no longer being applied. If a role has 'edit domain nodes' they should be able to edit all of the nodes in that domain.. but they are no longer able to. It seems to be ignoring those settings entirely. Domain Access Advanced also exhibits that issue. I finally remembered that's why I wasn't using it.

Ken, in #22 you stated "The best solution for this in D6 is to use hook_domainrecords() to ensure that the proper permissions are written. In this case, insert an a TAC grant when taxonomy terms are empty, or an 'all' grant."

In #24 xjm indicates that's not a global solution.. BUT.. would it make sense for me to try to write a small module that implements that for my use case? I'm not much of a developer but with a little guidance I'd be happy to give it a try.

I disagree with #24. I think a simple bridge between TAC and DA as a stand-alone module is perfectly fine.

Okay. I'll give it a shot. Should your comment in #22 on using hook_domainrecords() be enough to get me going? So create a small module that uses hook_domainrecords() to force a TAC grant (or all grant) when a node is saved?

I think I can get the information I would need to do that from the node_access section in api.drupal.org. And I'll need to learn about hook_domainrecords() from DA.

Thanks. If you don't mind leaving this open for a bit I'd like to give it a try and possibly post some code if I'm struggling with it.

Yes. Sure.