Fork me on GitHub

Where are permission checks /not/ necessary?  Bottom

  • I've seen a lot of modules that have permission checks in virtually every function, but this works out to be needless in some cases (ie, double-checking security).

    Which functions would get permission checks in a "perfect" module? Every function that interacts with the database would obviously need a permission check, but beyond that, which other functions also require a check?

    If a module consisted of the following basic admin functions, which would require a permission check?

    pnadmin.php

    Code

    module_admin_main()
    module_admin_view()
    module_admin_new()
    module_admin_create()
    module_admin_modify()
    module_admin_update()
    module_admin_delete()
    module_admin_modifyconfig()
    module_admin_updateconfig()

    pnadminapi.php

    Code

    module_adminapi_create()
    module_adminapi_update()
    module_adminapi_delete()
    module_adminapi_getlinks()


    Less processing is good!

    :)



    edited by: alarconcepts, Jun 29, 2007 - 05:01 PM
  • I've struggled with this one myself, but come to the conclusion that all functions should have at least the basic perm check of 'Overview' if not only because I can not assume that the module should be available to anyone of any given site.

    I think the web master needs as much control as possible when it comes to perms, I've seen too many posts over time here where people want to restrict access to the simplest of modules but can't because the author assumed that it was a public module.

    alarconcepts

    Less processing is good!


    The Example module routinely quotes 'Security check - important to do this as early as possible to avoid potential security holes or just too much wasted processing.'... I assume this to mean 'why run any code if they shouldn't even be there.

    Just my 2 cents.

    --
    Under Construction!
  • Thanks for your input.

    It's a good idea to go more strict than OVERVIEW with admin functions, since users shouldn't be able to access admin functions. I've found myself using OVERVIEW permission level in an admin function a few times, but later determined that if a user needs access to a given function, then it is a user function after all (no matter where I wrote it initially). Admins can always access user functions anyway, so I would guess that this was the intention of the core team. It does seem ok though to leave out a security check on 'utility' functions (such as modname_admin_create()) where a pnConfirmSecAuthAction() or SecurityUtil::confirmAuthKey() is verified; the data is passed to the API where it is both verified and permission-checked.
  • 0 users

This list is based on users active over the last 60 minutes.