Abstract function for authorization check in admin pages

[johnjosephbachir]

This patch provides an abstract function to reduce code repetition in cases of authorization check, with accompanying user feedback throughout the codebase, such as this:

if ( ! current_user_can('manage_options') )
    wp_die(__('You do not have sufficient permissions to manage options for this blog.'));

I tried testing it, but actually couldn't think of a case that these checks accomodate for the menu.php/admin.php whitelist system takes care of all the unauthorized access cases that I tried.

I made the $full_message parameter in order to accomodate for messages with non-standard structure, but it turned out that they were all standard. The only slight exceptions were the couple of instances that were wrapped in <p> tags, which I deemed unnecessary in the rare case of simply telling a user that they aren't allowed to do something -- I figured these must have been formatted differently only for historical reasons and not because the behavior is actually different in that cast -- but correct me if I'm wrong.

So-- feel free to remove the $full_message logic (since it's not currently being used in any of the cases) -- or, maybe it will come in handy for plugin developers?

Let me know what you think!

[johnjosephbachir]

Gah-- didn't preview-- please excuse syntax and spelling errors in the ticket...

[dd32]

Translation functions must be passed a string.

__($message); will insert "$message" as a string into the translation file, Its not aware that t might be set to something else elsewhere.

sprintf() also needs to be used to concencate, ie. sprintf(__('Sorry but you cannot %1$s this blog'), $message); However combining multiple strings this way does not work in all languages, "Cannot manage options" may not always have the same structure within foriegn languages (In that, It cant just be plonked within another string without words being shuffled between them both)

Future release pending Developer feedback and/or workable patch

[johnjosephbachir]

How about this invocation then?

check_authorization('manage_options', __('You do not have sufficient permissions to manage options for this blog.'));

[azaozz]

DD32 is right, looks like we can either have a customized message or introduce a small function to check permissions and output a standard ('You do not have sufficient permissions to access this page') error message.

Don't think we need the complexity of a check_authorization() function that tries to construct custom messages, won't be useful for plugins too. And it becomes redundant if we have to pass the full message to it, better to leave it as is.

[johnjosephbachir]

I'm a bit lost-- what are you saying is more complex, one flavor of the abstracted function vs. another? Or the abstracted function vs. repeating the same code dozens of times?

[johnjosephbachir]

The reason I ask is, if it's the former, I would like to adopt my patch to your vision, and if it's the latter, then this will help me provide more helpful patches in the future.

[dd32]

How about this invocation then?

Would work.

Whilst i dont like repeating code.. The auth checks are simple if statements, It makes it obvious as to what happens. "If not current user can perform manage_options, then die with message BlahBlahBlah". I'd favour leaving them be for the simple fact of readable code.

[scribu]

I agree with dd32: you loose readability and you don't get much benefit: 1 line instead of 2.