Comments

ankur’s picture

Currently, there hasn't been any work to port this module to 5.0. If someone is willing to submit a patch, I will commit (or maybe just hand maintainership over to them).

-Ankur

energie’s picture

Title: 5.0 Version? » 5.0 Version
Status: Active » Needs review
StatusFileSize
new37.27 KB

Here is the almost completely rewritten source of the new 5.0 version of this module.

New hooks implemented: hook_node_grants(), hook_node_access_records() thus code that was managing the node access table has been thrown away. The hooks form_alter and nodeapi are reworked.

I've changed the persistent variable names everywhere since they are "automatically generated". One variable stores all the roles in an array instead of using the old variable naming format.
This will result an incompatibility between an existing 4.7 stuff and this version.

There is no necessary initial changes to take when enabling the module because of the new access system implemented in the core.
If someone disables the module and later enables it again those nodes that were updated meanwhile are loose the original node_privacy_byrole permission settings because node_access_write_grants() deletes the {node_access} record by default.

I removed some lines from the help hook in connection with the old explicit enable/disable mechanism.

  • I have not modified the node_privacy_byrole_upgrade.php and I have not created an update file.
  • readme, upgrade instructions should be modified

Hope it works correctly, testing needed.

Best regs,
Laci

mike stewart’s picture

I'd like to help test, but don't quite understand your write-up.

were there any database changes?

do i need to do anything special to install this in vanilla drupal 5.1 setup? it sounds like upgrade using your patch is not an option. implying if I apply the patch to the 4.7 NPBR module i can then install this to a vanilla version of 5.1. correct?

however, you also mentioned if there's existing nodes that were created with 4.7, it won't work. can you elaborate? (this is part of my confusion on how to install)

lastly, (and leastly), it sounds like there were changes to core, in terms of security. but I dont know what changes occurred, or where to find more info on this. if you have a link that documents the changes I'd find it useful - i dont want to spend hours researching as i tend to find too many things of interest. ;-)

brianpeat’s picture

I'm confused too. I ran the patch on the Head version, and uploaded it to my 5.1 install, but it doesn't show up in the Module list, so I can't even turn it on.

brianpeat’s picture

Yeah!

I have to do more testing, but I took a .info file from another module and created one for the patched Head version of this module. It worked!

brianpeat’s picture

crap.

It's installed, and it shows up in each node edit page...but the settings don't actually affect anything. My guess is there are some tables that aren't getting created (since there's no install file with this module).

any ideas?

luti’s picture

I've installed that module as well.

Based on your selection for a node, there appear table entries in a node_access table. So, brianpeat, don't jump into the conclusions. In particular as it seems you are not exactly an expert in those things - since the .info file was such a discovery for you... ;-)

Unfortunately, you are right that it doesn't work, as it is now... :-(

I've installed it to my test 5.1 site, and selected acces only for authenticated users. The page is displayed for anonymous users as well, unfortunately. I've tried to play a bit with manually entered node_access entries, but it seems it doesn't influence anything... So, I presume a problem is how Drupal decides weather to allow access or not (or, doesn't do that at all - I have the impression only the first entry which seems to allow access to any node for anyone is considered), and not the table entries themselves (they seemd to be there as expected).

energie’s picture

StatusFileSize
new37.61 KB

1. Documentation
http://api.drupal.org/api/5/function/hook_node_access_records
http://api.drupal.org/api/5/function/hook_node_grants

2. Patch
I used this manual to create the patch
http://drupal.org/diffandpatch#comment-147010
I haven't seen that the new files (.info) are not added automatically. Sorry.
New patch attached that contains the info file. No other modifications have been done.

3. There are no additional tables required by this module.

4. The records in the node_access table from previous (4.7) version are compatible with this module.

5. Variable names in the variable table, for instance: npbr_forview_%nodetype%_default_%roleid% are not compatible with the naming of the new versions' variables.
Thus settings of the module have to be re-applied by node-type.

6. Check if you have a line in the node_access table like this
0 | 0 | all | 1 | 0 | 0
If so, remove it. I will check the documentation and manuals to get sure if this module have to take care of it or not.

That is the reason to show a node to anonymous users not only to the granted roles.

luti’s picture

Yes, the record you are mentioning is the reason for module not to work properly. I've rebuilt permissions in between (after my previous post and came here now to publish my observations) using the "Administer" -> "Post settings" -> "Rebuild permissons" button, and the "problematic" record mentioned (0,0,all,1,0,0) was gone (together with a majority of NPBR records, but not all - interesting!). After this, nodes were not accessible anymore, until I've explicitely set permissions for every node!

I think this is not how it should be, as if no NPBR options are set, I believe the access shall be granted to anyone, like there is no NPBR module. By my opinion, this would also be the most comfortable solution (default should be to grant access to view nodes for any role or user and probably delete/edit permissions for author, as long as some other combinations are not selected by NPBR module explicitely).

Can someone more skilled in Drupal API suggest a patch to achieve that, please. Or, maybe a module settings, where site administrator could choose the behavior for a case when permissions for node are not specified (default: access all, access noone) would be even better?

Now, NPBR module sets for node author only a view permission, but not edit and delete as well. Should it be like that, and why is it so?

pkrasko’s picture

Category: feature » support

I used the most recent patch [npbr_20070213.patch] and applied it to node_privacy_byrole.module. I copied the directory into my drupal module directory and enabled it on the modules page. If I want to add new content or edit existing content I get this at the top of the page:

In order to properly enable the node_privacy_byrole module, you must also enable it in the module's settings page.

There isn't an entry on the settings page for node_privacy_byrole... What is going on here? I can't seem to get this module working on 5.0.

Help appreciated...

Phil

pkrasko’s picture

I used the most recent patch [npbr_20070213.patch] and applied it to node_privacy_byrole.module. I copied the directory into my drupal module directory and enabled it on the modules page. If I want to add new content or edit existing content I get this at the top of the page:

In order to properly enable the node_privacy_byrole module, you must also enable it in the module's settings page.

There isn't an entry on the settings page for node_privacy_byrole... What is going on here? I can't seem to get this module working on 5.0.

Help appreciated...

Phil

pkrasko’s picture

Alright it seems the problem is my patch isn't being applied. I have tried patch -p0 < npbr_20070202.patch and patch patched_file.module < npbr_20070202.patch on BSD and on Windows. The patched file is empty.

This is what I get spit back to me:

Hmm... Looks like a unified diff to me...
can't find file to patch at input line 10
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|? node_privacy_byrole.info
|? npbr_20070202_drupal5.patch
|Index: node_privacy_byrole.module
|===================================================================
|RCS file: /cvs/drupal-contrib/contributions/modules/node_privacy_byrole/node_pr
ivacy_byrole.module,v
|retrieving revision 1.27.2.2
|diff -u -F^f -r1.27.2.2 node_privacy_byrole.module
|--- node_privacy_byrole.module 11 Aug 2006 09:33:12 -0000 1.27.2.2
|+++ node_privacy_byrole.module 2 Feb 2007 15:11:08 -0000
--------------------------
File to patch: patch.module
Patching file patch.module using Plan A...
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file patch.module.rej
Hmm... Ignoring the trailing garbage.
done

Does anyone have a patched version they can share? If I'm doing something wrong please let me know.

Thanks,

Phil

luti’s picture

StatusFileSize
new13.03 KB

I'll try to put the complete patched module archive here - for the first time, hope it will work ;-)

Arrrrrgh - that's what I got:

The selected file npbr_20070213_patched.tar.gz can not be attached to this post, because it is only possible to attach files with the following extensions: patch jpg jpeg gif png txt html doc xls pdf ppt pps odt ods odp.

So, the file is renamed into .doc, just rename it back (removing this extension), I hope the system will not screw it...

By the way, Drupal guys, don't you think it would be better simply to allow users to attach archives as well - as I see many of them here, "hidden" as patches mainly - this way, you are not preventing people to publish them, but just introduce a need for more explanations to be written and published, and I am noticing also quite some confusion due to that... (what at the end means just more time spent and more data to be published and transferred - so, what the benefit could be at the end)?!

luti’s picture

I see a file name has been modified during upload somehow - so, attachment above npbr_20070213_patched.tar_.gz_.doc should be renamed into npbr_20070213_patched.tar.gz first...

brianpeat’s picture

I installed the new patched version and while I can check lots of nice boxes, doing a search as an annoymous user still gets me into all the Blog node entries, even though they're all set to Authenticated (the node type and ALL the node entries). Guess I'll keep messing and see if I can figure out what's up.

luti’s picture

Read the posts above, brianpeat - it is discussed in #8 and #9... ;-)

I've tried (with the same patch applied), and there is no problem with me - I find a note with the same term when logged in, while anonymous search returns no results (for a node where permissions are set so).

brianpeat’s picture

Yeah, I wondered if posts #8 abnd #9 were needed with the last patch, but I haven't had time to try them. I'll try that today and see if it works.

brianpeat’s picture

No luck for me. I don't see that entry in my database (as far as I can tell) and I've rebuilt the permissions. I set the Blog node type to be Authenticated user only, and then I made sure the 1 of the previous test blog entries also was set to Authenticated for the View column...and it STILL shows up when you do an anonymous search.

brianpeat’s picture

UPDATE:

Aha! Thankfully this is just a test site as I discovered any NEW posts follow the rules. I just have a few test blog posts that I need to delete and things should be working. I did a test blog post just now and THAT post doesn't show up in an anonymous search, so it would appear the system works.

What does this mean for me? This test site is for a school and each class title is a Role. Students not in a certain class (who haven't taken it yet) aren't supposed to have access to the content. Instead of tying up the Taxonomy system using the the TAC module, I can feasibly create node types for each class and use this module to manage permissions. Yeah! and thanks for those who work on things way over my head so that I can get my work done.

jsimonis’s picture

So is this ready to be posted as a module so it can be easily downloaded and used?

luti’s picture

Seems to work in my test environment, so - yes, according to me.

lvadillo’s picture

I've changed the value in the node_access table "0 | 0 | all | 1 | 0 | 0" to "0 | 0 | all | 0 | 0 | 0" and the rest of nodes seem to behave properly.
Do someone knows how to make a NEW node readable by all as is was prior to Drupal 5.1?. It's a matter of putting all ones in the "read" instead of all zeroes. Don't know were to make this change and I need to keep with the old way.

Thank you very much for making this module available in 5.1, if not, no way for changing from an older version.

luti’s picture

Ivadillo,

have you tried to play a bit with default settings for new nodes?

I've set there some default node access rules (permissions for setting permissions, edit and view a node) for content types (settings are hardly visible, as the section is collapsed by default, it is for me at the top of a settings page), and any new node created gets such permissions automatically, when created (which you can of course manually override).

rcross’s picture

Any more progress on this? I'm also particularly interested in an upgrade path. This is the only module holding me back from upgrading an intranet site to the 5.x branch. I've got quite a bit of content inside the intranet, so the option of just "delete old posts and recreate them and it works" isn't feasible, and I also don't want to have to try and go reset permissions on each page to work with the new version either.

Thanks

ahaze’s picture

LUTi, thank you for working on this. I'm not a developer of any kind, just a user. I've just upgraded a test site from 4.7 to 5.1 and installed your patch. My site is mature and has lots of nodes, so hand editing them all isn't feasible- luckily, my previous view permissions from 4.7 are still working. However, I have a couple questions:

When I go to admin/content/types and edit a content type, npbr permissions settings are at the top, but this is what they say (emphasis mine):

Permissions For Permissions
Select by role which users can, by default, edit nodes of this type.

Default Edit Permissions
Select by role which users can, by default, view nodes of this type.

Default View Permissions
Select by role which users can, by default, view nodes of this type

Should these be changed to:

Permissions For Permissions
Select by role which users can, by default, set permissions for nodes of this type.

Default Edit Permissions
Select by role which users can, by default, edit nodes of this type.

Default View Permissions
Select by role which users can, by default, view nodes of this type.

Also, even though I have the module enabled, view permissions settings aren't there to me when I create new content or edit existing content. Any idea why? Thanks in advance.

luti’s picture

About the descriptions, I believe you are right.

It works to me as expected, but I didn't upgrade 4.7 for my test site (I did a fresh 5.1 install and copied a few pages of content manually, as there were some significant differencies in modules available at that time comparing to what I am using on my production site).

Are you sure it is not there? As by default it is collapsed in 5.1, and you see just a text "View-Edit Permissions".

ahaze’s picture

You're right, it was there, I just had a case of blindness. That being said, it sounds like it *should* work. But I ended up with this problem even though I disabled the 4.7 version of NPBR before installing 5.0.

jondoesdrupal’s picture

Hey folks, great job to all of you who've been working on this so far. This is the access module I've been looking for ever since I started using drupal - can't believe I've never seen it before.

Anyway, just a quick one as I'm about to head out the door, but I found that the permissions setting to define who was able to edit the permissions on a node wasn't working. I've tracked this down to the function _node_privacy_byrole_get_default_roles which is looking for the wrong variable name to pull back the roles.

Firstly, the action in parameter 2 was being specified as 'meta' when I believe it should be 'grant'?

Secondly, the function that called it, node_privacy_byrole_user_has_meta_perm, was defining the first parameter $type by calling $node-> type, however $node isn't in that function's scope. I've now changed it so that it is passed $node as a parameter by node_privacy_byrole_form_alter which obtains the $node object from $form['#node'].

It now looks to be working ok, but I'll check it over properly and see try help iron out any more issues so we can see about trying to make this into an official release.

Is the project maintainer keeping tabs on the progress here???

Cheers,

Jon

jondoesdrupal’s picture

I think there's a flaw in the design for this implementation in that it relies solely on the node_access table, whereas it needs to be storing the permissions itself separately and updating both sets of records.

This would be why when you rebuild the perms you lose everything - you truncate the table and then go to rebuild the perms structure, but you can't, because you just dropped all the data!

I think I've finally got my head around how all this works so I'll have a go at changing it myself and hopefully will have something to show for it in a day or two. Once that's sorted, I'll take a look at the 4.7 version and see if we can produce an upgrade path (would have thought it shouldn't be too hard).

Cheers,

Jon

jondoesdrupal’s picture

StatusFileSize
new13.78 KB

OK, here's what I've come up with which appears to work pretty nicely. It contains the following additions to the last package posted:

- fixed bug which meant that settings to define who could update the permissions on a node weren't taking.
- updated this to use it's own table rather than directly using node_access, as per the correct way demonstrated in the handbook's example. Rebuilding the node permissions now correctly leaves you with the right permissions in place. Permissions will be reset correctly on disabling the module and will return after on re-enabling the module.
- updated the text on the content types form to clear up confusion on which controls controlled what.

If you're doing a fresh install, this should work straight out the box. If you've got existing data using the previous incarnation of this module, you'll need to be careful and takes some manual steps before installing this version (you'll lose your existing perms if you just go ahead and enable it).

In short, the new table is exactly the same as node_access in structure and needs to be there so it can be used to populate node_access after installation, so you need to copy this table to the new one - node_privacy_byrole - before installing the module. If you have phpmyadmin, this is a simple as going to the node_access table, selecting operations, then copying the table. You may well have entries in the table from other modules which you would need to remove manually (but shouldn't do any harm if not).

I _think_ this same procedure will work for upgrades from 4.7 to 5, but I haven't tested it.

Another thing to note is that even if you're starting from scratch but are using an installation in which you have previously installed the module, you will have to remove the relevant line from the system table as without that being done it won't attempt to run the .install file.

Cheers,

Jon

jondoesdrupal’s picture

StatusFileSize
new13.81 KB

Further fixes - update wasn't working, .install script was missing a comma, and I've now included instructions and sql statements that _should_work for upgrading from 4.7 or the earlier version found here in this thread.

esadot’s picture

subscribe

FH’s picture

Bonjour,
j'ai installé ce module pour la version 5 comme indiqué.
pour toutes les nouvelles publications c'est ok
pour les autres il y a quelques problèmes: modifications prises en compte dans la table node_acces mais pas mises à jour dans node_privacy_byrole.
Quand on édite une publication, les modifications précédentes n'apparaissent pas et si on ne les reprécise pas, on remodifie node_acces involontairement

FH’s picture

Pour illustrer mon messge précédent, voici un extrait de ma base:
 bis_node_access   101  MyISAM   latin1_swedish_ci 13,1 Ko
 bis_node_privacy_byrole   96  MyISAM   latin1_swedish_ci 12,8 Ko

cordialement

jondoesdrupal’s picture

Translations......

----------------------------
Hello, I installed this module for version 5 as indicated. for all the new publications it is ok for the others there are some problems: modifications taken into account in the table node_acces but not updated in node_privacy_byrole. When a publication is published, the preceding modifications do not appear and if one them reprécise not, one remodifie node_acces involuntarily
----------------------------
To illustrate my messge preceding, here an extract of my base: bis_node_access 101 MyISAM latin1_swedish_ci 13,1 KB bis_node_privacy_byrole 96 MyISAM latin1_swedish_ci 12,8 KB

jondoesdrupal’s picture

FH, are you definitely using the latest one I submitted on the 15th? My previous one had faults with the update code which would cause what you described. If, so, do you get any SQL errors or anything?
----------------------------------------------
FH, employez-vous certainement le dernier que j'ai soumis sur le 15ème ? Mon précédent a eu des défauts avec le code de mise à jour qui causerait ce que vous avez décrit. Si, ainsi, obtenez-vous des erreurs ou quelque chose de SQL ?

FH’s picture

>FH, are you definitely using the latest one I submitted on the 15th? My previous one had faults with the >update code which would cause what you described. If, so, do you get any SQL errors or anything?

jondoesdrupal,
oui, j'ai utilisé cette dernière version, je n'ai pas de message d'erreur mysql, comme je l'ai dit pour tout article, image, page, etc. créé après l'installation du module, tout fonctionne, pour les anciens articles, etc, je ne peux pas modifier de façon stable les droits

merci

traduction :
yes, I used this last version, I do not have an error message mysql, as I said for all article, image, page, etc created after the installation of the module, all is ok, for the old articles, etc, I cannot modify in a stable way the rights
thank you

jondoesdrupal’s picture

It sounds like the upgrade procedure I provided might not have worked. When you update an existing node, it will issue UPDATE sql statements, so if an appropriate row does not exist already then nothing will change.

Did the SQL statements in my UPGRADE.txt file execute correctly?

As an investigation, take a node id and try looking to see if there is a matching row for it in node_access and node_privacy_byrole. Unfortunately I haven't really tested the upgrade procedure, so when I provided it it was meant as a best effort and starting point for anyone who needed to do it.
________________________________________________________________

Cela ressemble à du procédé de mise à niveau que j'ai fourni ne pourrais pas avoir travaillé. Quand vous mettez à jour un noeud existant, il des rapports de la MISE À JOUR SQL de question, ainsi si une rangée appropriée n'existe pas déjà puis rien ne changera.

Les rapports de SQL dans mon dossier d'UPGRADE.txt se sont-ils exécutés correctement ?

Comme recherche, prendre une identification de noeud et un essai regardant pour voir s'il y a une rangée assortie pour elle dans les node_access et le node_privacy_byrole. Malheureusement je n'ai pas vraiment examiné le procédé de mise à niveau, ainsi quand je l'ai fourni on l'a signifié pendant qu'un meilleur point d'effort et de départ pour n'importe qui qui a dû le faire.

FH’s picture

bonsoir,
j'ai suivi les instructions du fichier upgrade text
, sans erreur,
il y avait alors les mêmes lignes dans node_acces et node_privacy_byrole
ensuite, les deux tables ne se synchronisent que sur les node récents
Pour contourner ce problème, j'ai recréé un par un les 140 premiers nodes, ....

traduction:
good evening, I followed the instructions of the file upgrade text ,
without error,
there were then the same lines in node_acces and node_privacy_byrole then, the two tables are synchronized only on the recent node
To circumvent this problem, I recreated one by one, 140 first nodes,….

webchick’s picture

Category: support » task
StatusFileSize
new45.21 KB

This is just #31 in patch form. The only difference is I moved UPGRADING.txt into UPGRADE.txt, since it's confusing to have two text files about upgrading. Haven't tested yet, but I hate blindly downloading stuff without the diffs. ;)

webchick’s picture

Oooo.. interesting. Doesn't like # in the file name. ;)

webchick’s picture

Priority: Normal » Critical
Status: Needs review » Reviewed & tested by the community

I've been hammering on this all last night and today (fresh install, not an upgrade)... I'm sure there are spots where it could stand to be cleaned up in places, but it seems like this could be done after getting a 5.x release out the door.

This module is recommended in the handbook as the way to control view permissions from CCK: http://drupal.org/node/77475, and we are about a week and a bit away from a 6.x code freeze. I'd say it's critical to get *something* available for 5.x that people can use.

jondoesdrupal’s picture

Yes, the last one I upped here seems to work reliably for me but I wanted to try and get it into CVS before making any more changes (tidying it up, improving the documentation, etc) so issues could be tracked more clearly.

I've tried contacting the maintainer a few times now but I've not heard back. Not quite sure how to proceed from here......

jondoesdrupal’s picture

Have just submitted a request to be given access to this project so I can get this code committed.

jondoesdrupal’s picture

Assigned: Unassigned » jondoesdrupal
Status: Reviewed & tested by the community » Closed (fixed)

Version 5 module has been committed and should be out in a few hours when the packaging script run.

Please use that and raise fresh issues with any findings.