Access is always denied [#1984124]

As far as I can tell access is almost always denied.

I'm second guessing myself because apparently 140 people are using 7.x-1.3 without trouble but I'm pretty sure.

The database has a column fiid, which is for the field instance id, however the module is storing the field id in there, not the instance id, so when access is checked using the instance id it generally doesn't match the field id that is stored.

The only way I can see it working for people is if they don't reuse fields and the field instance id is always the same as the field id.

Marking critical as the module doesn't really work with this bug.

Comment	File	Size	Author
#10	profile2_privacy-store_fiid-1984124-10.patch	3.37 KB	mariacha1
#10	interdiff.txt	1.4 KB	mariacha1
#7	profile2_privacy-store_fiid-1984124-7.patch	2.9 KB	mariacha1
#6	fieldid_match_config.png	41.43 KB	mariacha1
#6	fieldid_match_user.png	31.53 KB	mariacha1
#6	fieldid_not_match_config.png	42.72 KB	mariacha1
#6	fieldid_not_match_user.png	31.35 KB	mariacha1
#6	fieldid_not_match_front.png	5.44 KB	mariacha1
#6	fieldid_match_front.png	4.69 KB	mariacha1
#5	profile2_privacy-store_fiid-1984124-5.patch	2.24 KB	rooby
#3	permissions.png	45.68 KB	mariacha1
#3	empty_page.png	10 KB	mariacha1
#1	profile2_privacy-store_fiid-1984124-1.patch	1.42 KB	rooby

Comments

Comment #1

rooby commented 1 May 2013 at 13:25

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

Status	File	Size
new	profile2_privacy-store_fiid-1984124-1.patch	1.42 KB

This patch fixes it for me.

Comment #2

rooby commented 1 May 2013 at 13:27

The downside is it means you have to re-save all your setting.

So I guess this will need an update function to update all the settings for people.

I will leave as needs review for now to get the functionality reviewed and I will add the update function in the next day or two.

Comment #3

mariacha1 commented 3 May 2013 at 17:51

Status:

Needs review

» Needs work

Status	File	Size
new	empty_page.png	10 KB
new	permissions.png	45.68 KB

This patch loads the profile view page, but doesn't show me any fields. I am using the "main" profile and am using fields where the field id and instance id match.

Permissions

I just get the "Main Profile" title.

Blank page

Comment #4

rooby commented 5 May 2013 at 01:29

I'm going to check this out as I have seen it too (strange as I tried it on one site and it worked but on another it didn't).

Comment #5

rooby commented 8 May 2013 at 12:10

Status:

Needs work

» Needs review

Status	File	Size
new	profile2_privacy-store_fiid-1984124-5.patch	2.24 KB

Here is a new version that should work properly.

Comment #6

mariacha1 commented 17 May 2013 at 00:38

Status:

Needs review

» Needs work

Status	File	Size
new	fieldid_match_front.png	4.69 KB
new	fieldid_not_match_front.png	5.44 KB
new	fieldid_not_match_user.png	31.35 KB
new	fieldid_not_match_config.png	42.72 KB
new	fieldid_match_user.png	31.53 KB
new	fieldid_match_config.png	41.43 KB

Your patch looks good to me. Here's a few screenshots to confirm.

1) The "Name" field only exists once, so its field id matches its field instance id.
Here's the config:
Config
Here's the user page:
Config
Here's what the anonymous user sees:
Config

2) The "Last Name" field exists in another form, so its field id does not match its field instance id.
Here's the config:
Config
Here's the user page:
Config
Here's what the anonymous user sees:
Config

I'm not going to set this to RTBC, though, because it will need an update hook, as you mentioned, to be properly patched to be ported. I'd be happy to take a stab at that if you won't have time within the next few weeks.

Comment #7

mariacha1 commented 7 June 2013 at 01:29

Status:

Needs work

» Needs review

Status	File	Size
new	profile2_privacy-store_fiid-1984124-7.patch	2.9 KB

Here's the latest version of the patch with an update hook. It re-rolls patch from #5 into the latest dev version.

Comment #8

rooby commented 7 June 2013 at 03:47

Thanks for the updated patch.

I have a couple of comments:

+++ b/profile2_privacy.install
@@ -86,3 +86,15 @@ function profile2_privacy_field_schema($field) {
+function profile2_privacy_update_7001() {
+  $query = 'UPDATE {profile2_privacy_fields} SET fiid = ' .
+    '(SELECT id FROM {field_config_instance} ' .
+    ' WHERE {field_config_instance}.field_id = {profile2_privacy_fields}.fiid ' .
+    ' AND {field_config_instance}.entity_type = \'profile2\')';
+
+  db_query($query);

The update function number should be 7101.

See https://api.drupal.org/api/drupal/modules!system!system.api.php/function... for further information. I might be able to simplify the explanation if that one isn't clear.

A couple of things regarding the sql:
* You can use double quotes around the sql string so that you don't have to escape the internal quotes.
* If you use table aliases it simplifies things a bit.
* You can make it a single string.

For example (there is no Druapl standard for indenting sql though so it comes down to personal preference, you don't have to do it this way):

$query = "UPDATE {profile2_privacy_fields} ppf
             SET ppf.fiid = (SELECT id
                               FROM {field_config_instance} fci
                              WHERE fci.field_id = ppf.fiid
                                AND fci.entity_type = 'profile2')";

Functionality wise, I'll try to test it soon.
The main thing to test is having multiple profiles that use multiple instances of the same field.
We might get into trouble when a single fid has multiple ffids.

Comment #9

mariacha1 commented 7 June 2013 at 15:24

Status:

Needs review

» Needs work

Thanks @rooby! Lots of helpful stuff there, and I'll make changes asap.

It came to me in the night, too, that I neglected to test against multiple profiles with the same field, and, sure enough, this is an issue. I'll need to change the reference to the "entity_type" to instead pull in the "bundle" or possibly refer to the profile id.

Setting back to needs work.

Comment #10

mariacha1 commented 14 June 2013 at 01:17

Status:

Needs work

» Needs review

Status	File	Size
new	interdiff.txt	1.4 KB
new	profile2_privacy-store_fiid-1984124-10.patch	3.37 KB

Here's another attempt at an update hook. This completely recreates the original table by dumping all the new values into a temporary table, dropping the original table, and renaming it. I have a feeling this is not the ideal way to do this, but I believe it's less system-intensive doing all the updates in mysql. Criticism is welcome!

I'm attaching a patch and an interdiff between this and #7, although I see now that I've created it that it's not terribly helpful. :)

Comment #11

TBarina commented 23 July 2013 at 16:36

I've applied the path in #10 and now it all works perfectly here.
Many thanks.

Comment #12

randallknutson commented 22 October 2013 at 20:04

Status:

Needs review

» Reviewed & tested by the community

Patch worked for me and the update hook worked as well. This should be good to go and is critical since the main module just doesn't work without it.

Comment #13

jhedstrom

English

Portland, OR

commented 10 December 2013 at 01:56

Issue summary:	View changes
Status:	Reviewed & tested by the community	» Fixed

Looks like this got committed back in October.

Comment #14

24 December 2013 at 02:00

Status:

Fixed

» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Access is always denied

Comments

Comment #1

Comment #2

Comment #3

Comment #4

Comment #5

Comment #6

Comment #7

Comment #8

Comment #9

Comment #10

Comment #11

Comment #12

Comment #13

Comment #14

News items

Our community

Documentation

Drupal code base

Governance of community