[pmwiki-devel] minor(?) inefficiency in CondAuth function

[Patrick R. Michaud]

On Mon, Feb 26, 2007 at 07:39:01AM +0100, Thomas Pitschel wrote:
> I just came again across the authentication functions and have 
> seen the line
> 
>     return (boolean)RetrieveAuthPage($pn, $level, false, READPAGE_CURRENT);
> 
> in function CondAuth in pmwiki.php.
> 
> Isn't a bit waste to retrieve the page content just for finding out 
> whether there the user is authenticated on the specified level? 

Since passwords and permissions are held in the individual page
files, the only way to know if a person is to authorized for
a given page is to read the page file.  

> If there are many (:if auth ... :) clauses in the markup, this 
> slows  down the server response unnecessarily.

Yes, but what if a page has clauses like...?

    (:if auth read SomeOtherPage:) 

We cannot simply use read permission on the current page -- we 
have to check permissions for SomeOtherPage (and this requires
reading the page file for SomeOtherPage).

However, the results of any authorization checks are cached...
so it wouldn't be at all difficult to get CondAuth to check
the cache before making the call to RetrieveAuthPage.  Then
repeated (:if auth ...:) clauses would result in page reads
only for pages that haven't already been checked.

> As I am currently working on an alternative auth module, my 
> preferred solution would be to have an additional permission 
> check handle similar to $AuthFunction, but now with two arguments 
> (authprompt always false):
> 
> $CurrUserHasPermFor($pagename, $level).
> ...
> It's is of course uncritical though - one can always define 
> a custom Conditions['auth'] function and do whatever optimization 
> one likes.

You're correct that one can always set a custom Conditions['auth']
function; for that reason I think I'll avoid the extra
$CurrUserHasPermFor indirection, at least for now.  If we
come up with a strong reason to need it, we can add it later.

Thanks!

Pm