[D6] Access unpublished

Hi. Please read all the following and the links provided as this is very important information about your CVS Application:

Drupal.org has moved from CVS to Git! This is a very significant change for the Drupal community and for your application. Please read the following documentation on how this affects and benefits you and the application process:
Migrating from CVS Applications to (Git) Full Project Applications

• The status of this application will be put to "postponed" and by following the instructions in the above link, you will be able to reopen it.
• Or if your application has been "needs work" for more than 5 weeks, your application will be marked as "closed (won't fix)". You can still reopen it, by reading the instructions above.

As hint, it seems that you haven't yet correctly configured your e-mail address in git which means that the commits do not get assigned to your account. Have a look at the "Git access" local task below edit in your profile to see how to configure it properly. You are currently using an internal e-mail address (webserver2.icc something)

- You're .info file is missing a bit information, you should add the core version, package and description.

- // $Id$
This line isn't necessary anymore, git doesn't use it.

- drupal_set_message(t('Access unpublished') . ': ' . access_unpublished_get_access_url($node) . '');
You should rewrite and use a single translatable string with placeholders. Something like this:

drupal_set_message(t('Access unpublished: <a href="@unpublished_url">@unpublished_url</a>', array('@unpublished_url' => access_unpublished_get_access_url($node)));

Or something along those lines. You might also want to rework the text a bit.

- return md5($nid . variable_get('access_unpublished_privatekey', ACCESS_UNPUBLISHED_DEFAULTPRIVATEKEY));
I'm wondering if that is secure enough. If someone forgets to update the privatekey, this can be easily tricked.
What about generating a random string in hook_install()? That is similar to what Drupal 7 is doing to protect cron.php access. See http://api.drupal.org/api/drupal/modules--system--system.install/functio.... You could either backport the used helper functions to D6 or use something simpler like http://ch.php.net/manual/en/function.uniqid.php

Assigning for review.

Hi,

Your module looks good.

Here are a few suggestions, but I'll assign this as RTBC btw.

You're defining ACCESS_UNPUBLISHED_QUERYSTRING as constant. Wouldn't it be better (and more secure) to use variable_get() and let the administrator choose which string to use? That'd mean using check_plain() everytime you're retrieving it, of course.

  return md5($nid . variable_get('access_unpublished_privatekey', ACCESS_UNPUBLISHED_DEFAULTPRIVATEKEY));

Just as you're doing in your .install file, don't you think it'd be better to use uniqid() here? Even though it shouldn't ever be used, this is still better.

Well, by the way, you can do these changes later, as they are not essential, but would be a great addition.

You're using variable_get() to return the value. Value which you've defined through your .install file, so the constant should never be used by the way. If it is ever used, that means that the variable is empty, so it is using a default value (and defining this value in the table), which will be retrieved the next time by variable_get(). This new value should not be as simple as "abcdef", which is why I'm proposing you to use uniqid().

Uh, my bad. You're absolutely right, I confused several things together and said something totally wrong. Your way is indeed what I was thinking about. Sorry for thinking the wrong way.

Well, we just need someone with the right permission to approve your application now :-)

Hi aberg,

Thanks for your contribution and welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. As someone who has recently completed an application, your input would be especially useful in the code review group as we work to improve this process.

I am giving credits to the users who participated in this issue.