Making #access required on forms
Gerhard brought this up on the security team list, but it seems like it's worth broader discussion: --- Hi there, as more an more people use Drupal to provide non-traditional Webpages (e.g. providing services using Ajax, Flex, ...) our traditional access permission checks in hook_menu are less than ideal. For example, you can use drupal_execute to conveniently create content or anything else. However, no check for access permissions is done since this only happens in the menu hook for node/add/whatever. I therefore propose to push for D7 that the #access parameter on forms be made mandatory. Opinions? --- My initial thought on the downside is that this has implications for people who are using drupal_execute() to perform programmatic tasks (node/block/etc. creation, etc.); they would no longer work unless the script switched to user with the proper credentials (so we should probably get a nice user switching API function in core). It is also something in the upgrade steps that, if missed, will cause forms to completely disappear which is bound to result in support requests. On the other hand, it would provide extra security and would be akin to the way we force menu callbacks to provide an access control or they don't appear for anyone. It might also help us clean up some nasty places in core (node form, I am looking at you) where we have if (user_access(...)) hard-coded. So, I echo: opinions? :) -Angie
On Fri, Oct 24, 2008 at 12:00 PM, Angela Byron <drupal-devel@webchick.net>wrote:
Gerhard brought this up on the security team list, but it seems like it's worth broader discussion:
---
Hi there,
as more an more people use Drupal to provide non-traditional Webpages (e.g. providing services using Ajax, Flex, ...) our traditional access permission checks in hook_menu are less than ideal.
For example, you can use drupal_execute to conveniently create content or anything else. However, no check for access permissions is done since this only happens in the menu hook for node/add/whatever.
I therefore propose to push for D7 that the #access parameter on forms be made mandatory.
Opinions?
---
My initial thought on the downside is that this has implications for people who are using drupal_execute() to perform programmatic tasks (node/block/etc. creation, etc.); they would no longer work unless the script switched to user with the proper credentials (so we should probably get a nice user switching API function in core). It is also something in the upgrade steps that, if missed, will cause forms to completely disappear which is bound to result in support requests.
On the other hand, it would provide extra security and would be akin to the way we force menu callbacks to provide an access control or they don't appear for anyone. It might also help us clean up some nasty places in core (node form, I am looking at you) where we have if (user_access(...)) hard-coded.
So, I echo: opinions? :)
-Angie
I don't think that it should be a requirement or forced on developers. I do think it should be recommended. The processing, submission, and validation logic will have to be updated per form, especially if #access is used at the element level. #access is there and has been there. If someone has a good idea of a way to use it more effectively, document it and promote the approach.
On Fri, 24 Oct 2008 12:00:33 -0400, Angela Byron <drupal-devel@webchick.net> wrote:
Gerhard brought this up on the security team list, but it seems like it's worth broader discussion:
---
Hi there,
as more an more people use Drupal to provide non-traditional Webpages (e.g. providing services using Ajax, Flex, ...) our traditional access permission checks in hook_menu are less than ideal.
For example, you can use drupal_execute to conveniently create content or anything else. However, no check for access permissions is done since this only happens in the menu hook for node/add/whatever.
I therefore propose to push for D7 that the #access parameter on forms be made mandatory.
Opinions?
---
My initial thought on the downside is that this has implications for people who are using drupal_execute() to perform programmatic tasks (node/block/etc. creation, etc.); they would no longer work unless the script switched to user with the proper credentials (so we should probably get a nice user switching API function in core). It is also something in the upgrade steps that, if missed, will cause forms to completely disappear which is bound to result in support requests.
On the other hand, it would provide extra security and would be akin to the way we force menu callbacks to provide an access control or they don't appear for anyone. It might also help us clean up some nasty places in core (node form, I am looking at you) where we have if (user_access(...)) hard-coded.
So, I echo: opinions? :)
-Angie
I have long argued that any part of a form that should only appear for some users should *always* be added, and then controlled with #access, and the validate and submit handlers must be designed to handle either case regardless of permission settings. That makes forms much easier to form_alter in useful ways. I've been bitten by modules not doing that, or not being able to handle a form value that is #access = FALSE, way too many times, especially by OG. We may need to tweak the internals of FAPI a little so that it handles #access = FALSE in a consistent manner to make the submit hooks easier. Of course, having to specify #access everywhere would be a nightmare. So I'd propose: - *All* FAPI/drupal_render() elements must support #access. A validate/submit callback that does not function properly with #access set true or false is by definition buggy. - All elements define whether #access defaults to TRUE or FALSE if not specified. - For the main form itself, #access defaults to FALSE. For everything else, it defaults to TRUE. - Establish a standard to handle conditional form adds via #access, not via an if statement. Using an if(user_access()) is treated as a bug. If that causes bugs anywhere, then the bugs get fixed per the first item. - Document the hell out of the above so that people know the Power of the #Access side. Does that sound like a reasonable balance? In the degenerate case for a typical form, it's one extra line to add an #access => TRUE or #access => user_access() to each form. Of course, one alternative is to have an #access_callback and #access_arguments property pair a la menu API, but I don't know if that's overkill. :-) --Larry Garfield
Regarding
as more an more people use Drupal to provide non-traditional Webpages (e.g. providing services using Ajax, Flex, ...) our traditional access permission checks in hook_menu are less than ideal.
For example, you can use drupal_execute to conveniently create content or anything else. However, no check for access permissions is done since this only happens in the menu hook for node/add/whatever.
I therefore propose to push for D7 that the #access parameter on forms be made mandatory. Sometimes I want to be able to create content for a user behind the scenes but not allow them the ability to directly create the content. One of the strong points about Drupal is it makes a great framework that I can extend as developer. If you really push this through then I think the permission to create a content type should be separate from the ability to use the form to create that content.
Steve Ringwood
On Fri, Oct 24, 2008 at 10:33 AM, Steve Ringwood <nevets@mailbag.com> wrote:
Sometimes I want to be able to create content for a user behind the scenes but not allow them the ability to directly create the content.
That would still be possible by impersonating another user[1] (i.e. uid 1) and then setting the node author name to a different user. In general, I'm in favor of requiring #access at the base level of every form. I don't have a strong feeling about it on individual fields, but I feel like Larry's proposal is pretty strong. Regards, Greg [1] http://drupal.org/node/218104 -- Greg Knaddison Denver, CO | http://knaddison.com | 303-800-5623 Growing Venture Solutions, LLC | http://growingventuresolutions.com
On Fri, Oct 24, 2008 at 10:46 AM, Greg Knaddison - GVS <Greg@growingventuresolutions.com> wrote:
On Fri, Oct 24, 2008 at 10:33 AM, Steve Ringwood <nevets@mailbag.com> wrote:
Sometimes I want to be able to create content for a user behind the scenes but not allow them the ability to directly create the content.
That would still be possible by impersonating another user[1] (i.e. uid 1) and then setting the node author name to a different user.
There's also an issue to add a core function for safely impersonating users: http://drupal.org/node/287292
Quoting Steve Ringwood <nevets@mailbag.com>:
I think the permission to create a content type should be separate from the ability to use the form to create that content.
Isn't that the case already. Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
I just visited Packt Publishing's award page and found that Earl Miles (merlinofchaos) was voted as one of the Most Valued People from Open Source Content Management Systems (try wearing that title on a T-shirt!). There are other projects besides Drupal listed. What I found interesting is that most of the MVPs for projects were the projects' lead/founder. Perhaps that says something about Drupal truly being community driven. Link at: http://www.packtpub.com/article/open-source-cms-most-valued-people-announced . PacktPub.com <http://www.packtpub.com> has been accepting MVP nominations since early July and for the majority of Content Management Systems, there were a number of candidates that received enthusiastic support. This demonstrates how many different people are key to the sucess of a CMS and how difficult it is to select an individual as the person who has contributed the most. The following list of names were put forward by members of the Content Management System's development team and community and represent the exceptional support, guidance, and sheer amount of time that the MVPs have given up to support the development and growth of the respective CMS. Congratulations to those who were selected as the first annual Open Source CMS MVPs. BryanSD
On Mon, Oct 27, 2008 at 1:39 PM, Bryan Ruby <bryan@cmsreport.com> wrote:
I just visited Packt Publishing's award page and found that Earl Miles (merlinofchaos) was voted as one of the Most Valued People from Open Source Content Management Systems (try wearing that title on a T-shirt!). There are other projects besides Drupal listed. What I found interesting is that most of the MVPs for projects were the projects' lead/founder. Perhaps that says something about Drupal truly being community driven. Link at: http://www.packtpub.com/article/open-source-cms-most-valued-people-announced ...snip...
Well deserved. Earl not only pioneered some valuable modules which help tie everything together, but also has been seeing them through, with proper maintenance and releases, always keeping them as reliable as core.
Congrats to Earl! Much deserved! -Laura On Oct 27, 2008, at 5:39 AM, Bryan Ruby wrote:
I just visited Packt Publishing's award page and found that Earl Miles (merlinofchaos) was voted as one of the Most Valued People from Open Source Content Management Systems (try wearing that title on a T-shirt!). There are other projects besides Drupal listed. What I found interesting is that most of the MVPs for projects were the projects' lead/founder. Perhaps that says something about Drupal truly being community driven. Link at: http://www.packtpub.com/article/open-source-cms-most-valued-people-announced .
PacktPub.com <http://www.packtpub.com> has been accepting MVP nominations since early July and for the majority of Content Management Systems, there were a number of candidates that received enthusiastic support. This demonstrates how many different people are key to the sucess of a CMS and how difficult it is to select an individual as the person who has contributed the most.
The following list of names were put forward by members of the Content Management System's development team and community and represent the exceptional support, guidance, and sheer amount of time that the MVPs have given up to support the development and growth of the respective CMS.
Congratulations to those who were selected as the first annual Open Source CMS MVPs.
BryanSD
There is something about the name "Earl Miles" that sounds very dignified, and with all his good work I think we should knight the MVP so I can him, Sir Earl Miles Plus, I think it would be cool to see him slaying code with a sword! Who's with me? =)
One of the people with the highest impact on Drupal functionality, usability, one of the reasons many people stay with the Drupal CMS Framework. Gigantic. Victor Kane http://awebfactory.com.ar On Mon, Oct 27, 2008 at 1:00 PM, Ryan Cross <drupal@ryancross.com> wrote:
There is something about the name "Earl Miles" that sounds very dignified, and with all his good work I think we should knight the MVP so I can him, Sir Earl Miles Plus, I think it would be cool to see him slaying code with a sword! Who's with me? =)
Congratulations, Earl. Well deserved praise. /me now waiting for Earl to appear on the best-sellers list. *wink wink* ;-) ..chris
On Mon, Oct 27, 2008 at 1:00 PM, Ryan Cross <drupal@ryancross.com> wrote:
There is something about the name "Earl Miles" that sounds very dignified, and with all his good work I think we should knight the MVP so I can him, Sir Earl Miles Plus, I think it would be cool to see him slaying code with a sword! Who's with me? =)
Congratulations, Earl! Amy :)
as more an more people use Drupal to provide non-traditional Webpages (e.g. providing services using Ajax, Flex, ...) our traditional access permission checks in hook_menu are less than ideal.
I don't see a problem here. It is up to the service that exposes the functionality to control access. It feels like this is a solution in search of a problem. Can we identify some more concrete use cases or an SA that would have been avoided had we implemented this?
For example, you can use drupal_execute to conveniently create content or anything else. However, no check for access permissions is done since this only happens in the menu hook for node/add/whatever.
thats a feature. use reponsibly, just like the rest of our php code. @Crell - OG has problems dealing #access or other modules don't understand its use of #access? Yes, #access is incompletely implemented in core for users who have no access. see the hoops that KarenS jumped through at http://drupal.org/node/298440. So I agree that FAPI needs improving regardless of this proposal. Perhaps people would use #access more if it worked as expected. Same for #disabled (see http://drupal.org/node/227966)N
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Moshe Weitzman schrieb:
as more an more people use Drupal to provide non-traditional Webpages (e.g. providing services using Ajax, Flex, ...) our traditional access permission checks in hook_menu are less than ideal.
I don't see a problem here. It is up to the service that exposes the functionality to control access. It feels like this is a solution in search of a problem.
I've encountered the same problem for a second time today. The problem can be summarized as "submit a form in a way that does not involve rendering a html page".
Can we identify some more concrete use cases or an SA that would have been avoided had we implemented this?
- From the fact that I needed this functionality, I infer that there are others who need it too. I've found 111 invocations of drupal_execute in CVS for D5 alone. I guess that there are 2-10 SAs in there.
For example, you can use drupal_execute to conveniently create content or anything else. However, no check for access permissions is done since this only happens in the menu hook for node/add/whatever.
thats a feature. use reponsibly, just like the rest of our php code.
It is annoying if you want to do exactly that. To get around the problem of #access-less forms, I need to hook_form_alter #access to false for all of them and then one-by-one set permissions for the ones I need. Cheers, Gerhard -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) iEYEARECAAYFAkkCFfcACgkQfg6TFvELooQp7gCgnb7K7KEcV7wbgZmEUeXkBKTw R4AAni9wGxk5iuEj2GF9bfHaoOFT2W5Q =I9p8 -----END PGP SIGNATURE-----
On Fri, 24 Oct 2008 13:43:48 -0400, "Moshe Weitzman" <weitzman@tejasa.com> wrote:
@Crell - OG has problems dealing #access or other modules don't understand its use of #access?
I'd form_alter something to hide it, and rather than passing through its value unchanged (what I'd expect) it ended up not being set at all. I ran into similar issues with CCK, which seems to have its own form element structure that doesn't map to anything else so is a huge PITA to form_alter. :-) This is under D5, though, so I don't know if it's nicer in D6. I guess this is a branch of this thread to continue in the issue queue when next I try to form_alter stuff in weird ways. --Larry Garfield
so maybe if #access == false, an element gets it's value set to the #default_value and it is not rendered? just to make handling the form more consistent?
On Friday 24 October 2008 11:38:31 pm Darrel O'Pry wrote:
so maybe if #access == false, an element gets it's value set to the #default_value and it is not rendered? just to make handling the form more consistent?
That is what I would expect to happen, and 99% of the time is what I want to happen. :-) If that's not what happens now, uh, what does happen? -- Larry Garfield larry@garfieldtech.com
I'd change #type to 'value' and #value to #default_value. It's not as automatic as using #access On Sat, Oct 25, 2008 at 1:12 AM, Larry Garfield <larry@garfieldtech.com>wrote:
On Friday 24 October 2008 11:38:31 pm Darrel O'Pry wrote:
so maybe if #access == false, an element gets it's value set to the #default_value and it is not rendered? just to make handling the form more consistent?
That is what I would expect to happen, and 99% of the time is what I want to happen. :-)
If that's not what happens now, uh, what does happen?
-- Larry Garfield larry@garfieldtech.com
I've tried that, too. That also breaks sometimes depending on the module, especially for CCK fields. On Saturday 25 October 2008 2:33:35 am Earl Dunovant wrote:
I'd change #type to 'value' and #value to #default_value. It's not as automatic as using #access
On Sat, Oct 25, 2008 at 1:12 AM, Larry Garfield <larry@garfieldtech.com>wrote:
On Friday 24 October 2008 11:38:31 pm Darrel O'Pry wrote:
so maybe if #access == false, an element gets it's value set to the #default_value and it is not rendered? just to make handling the form
more
consistent?
That is what I would expect to happen, and 99% of the time is what I want to happen. :-)
If that's not what happens now, uh, what does happen?
-- Larry Garfield larry@garfieldtech.com
participants (17)
-
Amy Stephen -
andrew morton -
Angela Byron -
Bryan Ruby -
Chris Johnson -
Cog Rusty -
Darrel O'Pry -
Earl Dunovant -
Earnie Boyd -
Gerhard Killesreiter -
Greg Knaddison - GVS -
Larry Garfield -
Laura Scott -
Moshe Weitzman -
Ryan Cross -
Steve Ringwood -
Victor Kane